CLI: allow augur --help to run without DB configuration#3661
CLI: allow augur --help to run without DB configuration#3661mezo78902 wants to merge 1 commit intochaoss:mainfrom
Conversation
e0c3977 to
ecef22b
Compare
augur --help works without DB config
|
@MoralCode I opened a new minimal PR (#3661) that only changes |
yep, thats this PR, thanks! |
|
Can you provide some more detail on why this change was needed? this feels like a very structural change to the object that seems to be underlying almost every CLI command. Was it the importing of this module that is dependent on the database? |
Great question — The issue is that Click builds the help output by calling Some of those command modules have import-time side effects or transitive imports that touch database or config logic, so This change only defers importing the command module until the command is actually invoked. The change is isolated to CLI wiring and avoids touching DB, Redis, or runtime logic. |
Do you have a sense of how much of the CLI is impacted by this? if its only a couple files maybe its worth adjusting how the imports are structured so that click can do its job as intended. |
I actually just tested that exact approach by refactoring backend.py to move all top-level imports (tasks, celery, KeyClient) into local function scope. augur --help still failed with the same DB error. That suggests the issue is transitive and affects more than a couple of files—importing one CLI module often pulls in others that eagerly touch DB/config at import time. Refactoring imports file-by-file would likely turn into whack-a-mole. The lazy proxy approach here provides a safer, structural boundary that ensures --help works without impacting runtime behavior. |
|
Hi @MoralCode Happy to update the PR this way if that sounds reasonable. |
| def _load(self) -> click.Command: | ||
| if self._real_cmd is None: | ||
| module = importlib.import_module(self._import_name) | ||
| self._real_cmd = module.cli | ||
| return self._real_cmd | ||
|
|
||
| def _proxy_callback(self, *args, **kwargs): | ||
| # If click ends up calling callback directly, delegate | ||
| return self._load().callback(*args, **kwargs) | ||
|
|
||
| def invoke(self, ctx): | ||
| return self._load().invoke(ctx) | ||
|
|
||
| def get_params(self, ctx): | ||
| return self._load().get_params(ctx) | ||
|
|
||
| def get_help(self, ctx): | ||
| return self._load().get_help(ctx) |
There was a problem hiding this comment.
this still seems like it is effectively using importlib to import the command modules. I dont think this is likely to work since theres a lot of database dependencies at import time :/
|
Id be curious to know whether Having a demo of what your code outputs for the help text would also be nice to have as a proof of concept Im worried there may not be a good solution here besides fixing the underlying "database connection needed at import time" issue, which is a very major change |
|
Thanks — you’re right about the proxy still importing. I haven’t pushed the placeholder approach yet; I just tested it locally. I’ll update this PR to the placeholder-only-for-top-level-help version (no imports during augur --help, no short_help parsing) and add the demo output. |
is this going to make the help text less useful? |
A little, yes — it may be a little less informative because we’d lose the per-command short_help lines in the top-level listing. |
|
Just to clarify — I haven’t pushed the placeholder-based update yet. The current PR still reflects the lazy proxy implementation. Before updating the PR, I’d really appreciate your perspective — do you feel that omitting short_help is an acceptable tradeoff for ensuring --help works reliably without DB configuration? |
yeah lets do that. its not ideal but having a dynamic list of all the commands that exist is probably more helpful The other option is to put a copy of the help output in the docs somewhere as a workaround and tackle the long term solution to this at once (but i worry we will forget to follow up on that. In addition to moving ahead on this, id be curious how possible you think it might be to use AST-like methods to parse through the files to recover the short_help and re-attach them to their relevant help items. I suspect this should be a separate PR though |
ecef22b to
ceea310
Compare
augur --help works without DB config|
Hi @MoralCode, I’ve updated the PR to use the simpler placeholder-only approach we discussed. During top-level augur --help, get_command() now returns a minimal click.Command(name=...) so that help can render without importing any command modules. No imports occur during help generation. The normal execution path for augur remains unchanged — modules are still imported at invocation time as before. As agreed, this version intentionally omits per-command short_help lines in the top-level listing to avoid any import-time side effects. Here is the output in a fresh environment with no DB configuration: $ augur --help Augur is an application for open source community health analytics Options: Commands: Please let me know if this aligns better with your expectations. I’d be happy to explore an AST-based approach for restoring short_help in a separate follow-up PR. |
|
It may be worth including a message in this limited output To explain that the output is intentionally limited because there's no database access available and linking to the issue. |
Augur is an application for open source community health analytics
Options:
--help Show this message and exit.
Commands:
api Commands for controlling the backend API server
backend Commands for controlling the backend API server & data
collection workers
cache Commands for managing redis cache
collection Commands for controlling the backend API server & data
collection workers
config Generate an augur.config.json
csv_utils
db Database utilities
github Github utilities
jumpstart
tasks Commands for controlling the backend API server & data
collection workers
user Support for adding regular users or administrative users works without DB config
Signed-off-by: Hamza <mezohafez1@gmail.com>
ceea310 to
d26c6a4
Compare
|
Thanks for the suggestion — I’ve added a brief note to the help output explaining that it is intentionally limited when no database configuration is available, and linked to #3654 for context. Please let me know if this wording looks appropriate. |
Fixes #3654.
augur --help previously triggered eager imports of CLI command modules via click.MultiCommand.get_command(), which could initialize DB/config and exit before rendering help.
This change returns a lightweight lazy proxy command from get_command(), importing the actual command module only when invoked. short_help is parsed from command source as a best-effort to keep the top-level help informative without importing modules.
Scope is intentionally limited to a single file: augur/application/cli/_multicommand.py. Runtime behavior for actual command execution remains unchanged.