fix: eliminate duplicate Configuration init and add --config CLI arg#264
Open
octo-patch wants to merge 1 commit intozilliztech:masterfrom
Open
fix: eliminate duplicate Configuration init and add --config CLI arg#264octo-patch wants to merge 1 commit intozilliztech:masterfrom
octo-patch wants to merge 1 commit intozilliztech:masterfrom
Conversation
…ixes zilliztech#197) The module-level config = Configuration() in configuration.py was instantiated on every import, causing a redundant second instantiation when cli.py explicitly created its own Configuration object. Remove the module-level instance since it is unused by any consumer. Move config initialization in cli.py to after argument parsing, and add a --config flag so users can point to a custom YAML file without editing the source, as discussed in the issue thread.
Collaborator
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: octo-patch The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
/assign @xiaofan-luan |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #197
Problem
When any CLI command is executed,
Configuration()is instantiated twice:config = Configuration()inconfiguration.pyfires as a side-effect of the import statement incli.py.cli.pyexplicitly callsconfig = Configuration()beforeinit_config().This is unnecessary: the module-level instance is never consumed by any code path —
online_query.pyandoffline_loading.pyaccessconfiguration.default_searcher,configuration.vector_db, etc., which are set only byinit_config().Solution
config = Configuration()module-level statement fromconfiguration.py.cli.pyto after argument parsing (avoids any startup overhead before the user's intent is known).--configflag to the CLI so users can specify a custom YAML path without editing source files, as suggested in the issue thread.Testing
Configuration.__init__is called exactly once when running a CLI command after the patch.main.pyare unaffected — they always instantiatedConfiguration()themselves and passed it toinit_config().