Skip to content
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

WebHost: ensure correct file type for config override #4351

Closed
wants to merge 1 commit into from

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

on prod the path to the gunicorn config file ends up in args.config_override. I don't really get why, since that's --config, not --config_override, but it does.

How was this tested?

For about two weeks on prod

Probably not the most elegant solution, but it is tested.

@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 9, 2024
@qwint
Copy link
Contributor

qwint commented Dec 9, 2024

argparse.parse_known_args() will match on just a prefix so the value in --config gets matched to --config_override
https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.parse_known_args
not sure a way to turn that off though

@qwint
Copy link
Contributor

qwint commented Dec 9, 2024

looks like

parser = argparse.ArgumentParser(allow_abbrev=False)

might work instead of the workaround in this PR

@black-sliver
Copy link
Member

The problem here is that it silently ignores some bad --config path/to/config..., so the other PR is definitely better,

@Berserker66 Berserker66 deleted the WebHost_config_override_type branch December 10, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants