Skip to content

Ignore unknown sys.argv #404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
isc-psulin opened this issue Sep 13, 2024 · 5 comments · Fixed by #405
Closed

Ignore unknown sys.argv #404

isc-psulin opened this issue Sep 13, 2024 · 5 comments · Fixed by #405
Assignees

Comments

@isc-psulin
Copy link

I really like the ability to override args from my config from the cli, but it breaks a lot of other things that also rely on sys.argv like pytest and uvicorn. I have tried setting cli_exit_on_error=False, but it doesn't seem to stop it from blowing up with args that are not declared in the model if I have cli_parse_args=True.

How can I have it just ignore args it does not know so I can use it with pytest, uvicorn, etc...?

@hramezani
Copy link
Member

@kschwab could you please take a look?

@kschwab
Copy link
Contributor

kschwab commented Sep 14, 2024

@hramezani this is similar request as #391. I've added a config option that will ignore unknown args.

The docs still need to be updated to cover the different use cases of mixing a CliSettingsSource with other parsers (e.g., pytest, uvicorn, etc.). I'll try to get back to it here in the next few days.

@isc-psulin
Copy link
Author

The solutions in #391 can work but require hard coding things to work with specific apps or just turn off CLI parsing if using certain apps. It is very common with Python apps to have another app load it - pytest, uvicorn, fastapi, memray - to just name a few I use off the top of my head, so these solutions are not ideal as they don't really acknowledge this common type of usage. I don't want application code that is specific to running tests so I am just turning off CLI parsing for these cases and will have to change the code if I need to use different launching apps.

@kschwab
Copy link
Contributor

kschwab commented Sep 17, 2024

@isc-patrick the similarity with #391 relates to mixing multiple parsers in a single app. This is a general problem. e.g., #405 will add an option to ignore unknown CLI arguments. This is a great request and a good feature to have 👍

However, while this resolves not blowing up for unknown arguments, it now also parses any common arguments. If pytest, uvicorn, etc. have a flag --enable int and CliSettingsSource also has a flag --enable dict, it will blow up. So, what constitutes "blowing up" is specific to the app and requires app specific logic to correctly resolve. This may mean turning off the CLI, integrating the CLI, ignoring unknown arguments, etc., but these are all app specific details.

@hramezani, unfortunately, I am tight on time this week. I've updated the docs with respect to the new flag. I'll try to get back to a broader summary of this issue and #391 in the docs at a later time.

@isc-psulin
Copy link
Author

@kschwab Thanks for the clarification. I mistook the disabling of the cli parsing via searching the argv[0] as that implementation. I completely agree that there is no way to avoid some of these issues because of the potentialof common arguments in different apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants