feat: Add option keepUnknownIndexes to retain indexes which are not specified in schema (#9857)#13
Conversation
… specified in schema (parse-community#9857)
Summary of ChangesHello @RahulLanjewar93, 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 enhances the configuration options available in the system by clarifying an existing schema setting and introducing a new database-level setting. The primary goal is to improve user control over database index management during schema migrations and to offer more granular control over MongoDB driver behavior when handling undefined values, ultimately leading to better maintainability and predictability. 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
|
![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 new configuration option keepUnknownIndexes. My review identifies a critical issue regarding the missing implementation for this new feature, which makes it non-functional as is. Additionally, I've provided a suggestion to improve code maintainability by sorting option properties alphabetically in the source file to keep it consistent with the auto-generated files and reduce future diff noise. Please see the detailed comments in the code.
| /* (Optional) Keep indexes that are present in the database but not defined in the schema. Set this to `true` if you are adding indexes manually, so that they won't be removed when running schema migration. Default is `false`. | ||
| :DEFAULT: false */ | ||
| keepUnknownIndexes: ?boolean; |
There was a problem hiding this comment.
This pull request adds the configuration option keepUnknownIndexes, but the implementation that actually uses this option seems to be missing. Without the corresponding logic changes to the schema migration process, this option will have no effect. This is critical for the feature to be complete. Could you please add the implementation?
| ignoreUndefined: { | ||
| env: 'PARSE_SERVER_DATABASE_IGNORE_UNDEFINED', | ||
| help: 'The MongoDB driver option to set whether to ignore undefined values in BSON documents.', | ||
| action: parsers.booleanParser, | ||
| }, |
There was a problem hiding this comment.
I see that ignoreUndefined has been moved here to maintain alphabetical order, which is great for readability. This is likely due to the build script that generates this file. To improve consistency and prevent noisy diffs in the future, it would be beneficial to also reorder the properties within the DatabaseOptions interface in src/Options/index.js to be alphabetical. This would make the source and generated files consistent.
2 participants
Pull Request
Issue
Closes: FILL_THIS_OUT
Approach
Tasks