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: disable abbreviations for argparse #4352

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

Alternative for #4351

How was this tested?

This one wasn't.

@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
Copy link
Contributor

@qwint qwint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is as much testing as I did, it looks right

>>> import argparse
>>> parser = argparse.ArgumentParser(allow_abbrev=False)
>>> parser.add_argument('--config_override', default=None,
...                     help="Path to yaml config file that overrules config.yaml.")
>>> parser.parse_known_args(["args", "here", "--config", "hello world", "--config_override", "test"])
(Namespace(config_override='test'), ['args', 'here', '--config', 'hello world'])

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming nobody relies on python WebHost.py --config path/to/config.yaml, this seems to be fine.

Should definitely put that into the release notes though.

@qwint
Copy link
Contributor

qwint commented Dec 10, 2024

my assumption was that this section of code only needed to parse --config_override because --config is already handled elsewhere, is that not the case?

@Berserker66 Berserker66 merged commit f79657b into main Dec 10, 2024
26 checks passed
@Berserker66 Berserker66 deleted the WebHost_config_abbreviation branch December 10, 2024 18:53
Berserker66 added a commit that referenced this pull request Dec 10, 2024
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