Conversation
fix config example typos - Convert AppConfig, ModelConfig, TelemetryConfig from dataclass to Pydantic models for native JSON serialization support - Add ConfigSignature to store complete AppConfig snapshot in Redis - Validate config on startup (read-only), save signature after successful init - Add `check_fields` option in PersistenceConfig to configure which fields to validate (default: ["supported_models"]) - Add --refresh-persistence CLI flag to clear existing data and start fresh - Add --force-refresh-persistence to skip confirmation prompt If config mismatch is detected, server exits with clear error message suggesting to either restore original config or use --refresh-persistence.
Summary of ChangesHello @xuanrui-L, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical safety feature to prevent data corruption in the TuFT server when using persistence. It implements a robust configuration validation system that checks for consistency between the running server's configuration and the previously persisted state. This ensures that incompatible configuration changes do not lead to silent data loss or unexpected behavior upon restart. The changes also include a migration of core configuration models to Pydantic for enhanced data validation and a clearer mechanism for users to manage persistence data when configuration changes are necessary. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
xuanrui-L
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a critical feature to prevent data corruption by validating the application configuration against a stored signature upon restart, especially when persistence is enabled. The implementation is robust, featuring a migration of configuration classes from dataclasses to pydantic for better validation, a user-friendly CLI for handling configuration mismatches, and comprehensive tests. My review focuses on improving consistency in the CLI's exit behavior, simplifying some redundant code, and fixing a small bug in the error message generation. Overall, this is an excellent contribution that significantly improves the reliability of the application.
|
/unittest |
|
/unittest |
| with pytest.raises(ConfigMismatchError): | ||
| validate_config_signature(config2) | ||
|
|
||
| def test_refresh_persistence_allows_restart_with_new_config(self, setup): |
There was a problem hiding this comment.
The test cases mentioned here should be modified/added to the integration test suite. Scenarios involving restarts and data cleanup are particularly important—both successful and failed restart cases need to be covered to ensure system reliability.
There was a problem hiding this comment.
This test currently aligns with the persistence unit test requirements. We can also add the corresponding config test coverage in the integration tests later—pls create an issue so we can track and address it.
- Changed persistence mode names in README and configuration files from `REDIS_URL` to `REDIS` and `FILE_REDIS` to `FILE` for consistency. - Updated CLI commands to replace `--refresh-persistence` with `tuft clear persistence` for clearing existing data. - Adjusted default namespace in persistence configuration to `persistence-tuft-server`. - Enhanced documentation to reflect these changes and improve clarity on usage.
3 participants
models for native JSON serialization support
check_fieldsoption in PersistenceConfig to configure which fieldsto validate (default: ["supported_models"])
If config mismatch is detected, server exits with clear error message
suggesting to either restore original config or use --refresh-persistence.