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

argparse subparser defaults in Python 3.9+ #2210

Closed
dgw opened this issue Nov 9, 2021 · 11 comments
Closed

argparse subparser defaults in Python 3.9+ #2210

dgw opened this issue Nov 9, 2021 · 11 comments
Labels
Bug Things to squish; generally used for issues High Priority Stale Mostly used for PRs that no longer work and need updating before re-review/merge. Tracking

Comments

@dgw
Copy link
Member

dgw commented Nov 9, 2021

Python seems to be making some changes to argparse's handling of subparser defaults.

  • The patch has landed in Python 3.9.8, and is causing CI failures for us now that GitHub Actions uses the new version.
  • Python 3.10 shows the same patch in "next", meaning it will likely break whenever 3.10.1 comes out.
  • Python 3.11.0 alpha 1 shipped this patch a month ago.

(All above verified via in-browser Ctrl-F for 'argparse' in the respective changelogs. The relevant patch is listed under the "Library" subheader.)

Our CI failures all come from numerous jobs in Sopel's test_cli_run.py suite, where in every case I looked at, the "custom" value passed in to validate the correct behavior is overridden by the option's default value. For example:

_______________________ test_build_parser_legacy_config ________________________
test/cli/test_cli_run.py:92: in test_build_parser_legacy_config
    assert options.config == 'custom'
E   AssertionError: assert 'default' == 'custom'
E     - default
E     + custom

Only two commits touched CPython's Lib/argparse.py after the tag date for 3.9.7, one of which appears to be irrelevant (it messes with some help stuff). That leaves python/cpython@a18d522, a backport of the patch for bpo-45235.

@jiasli
Copy link

jiasli commented Nov 11, 2021

This is great analysis and extremely helpful!

I saw (from https://bugs.python.org/issue45235) that the maintainer @rhettinger and @ambv have decided to roll back this change .

@dgw
Copy link
Member Author

dgw commented Nov 11, 2021

I saw (from bugs.python.org/issue45235) that the maintainer @rhettinger and @ambv have decided to roll back this change .

Indeed, that's why I'm letting #2211 and #2212 sit in limbo for now. If the change upstream is reverted, we don't need to scramble to release a patch. If this behavior change is still going to happen in a future Python version (I hope in 3.11 or 3.12, since 3.10 is already theoretically "stable" since last month), we will at least have time to plan for it and transition gracefully instead of being blindsided by sudden CI failures like this.

@jiasli I appreciate you opening the Azure CLI issue that ultimately got Guido's attention, too. 😸

@dgw
Copy link
Member Author

dgw commented Nov 11, 2021

For our own tracking purposes: The patch to revert this change is awaiting merge and backporting. python/cpython#29525

@dgw
Copy link
Member Author

dgw commented Nov 12, 2021

Revert merged to 3.11/main branch. Backports incoming; 3.9 is python/cpython#29531

@dgw dgw pinned this issue Nov 12, 2021
@dgw
Copy link
Member Author

dgw commented Nov 12, 2021

Backport now merged to 3.9 and 3.10 branches.

@dgw
Copy link
Member Author

dgw commented Nov 15, 2021

Python 3.9.9 released today, including the patch that reverts this change. I haven't found a way to poke the Actions team directly for an update, but will look harder when I have some more time.

Maybe I'll send a support ticket or Community thread, if that's all I can see. actions/python-versions doesn't accept issues, and it looks like PRs are supposed to be created via automation.

@dgw
Copy link
Member Author

dgw commented Nov 16, 2021

I will check tomorrow (technically today, but only because timezones are silly) if all is well by triggering some rebuilds for open, failing PRs. Python 3.9.9 should be available to our jobs now that it's released for use by actions/setup-python.

actions/runner-images#4521 says it should be baked into images (presumably any that use 3.9 by default) next week.

@dgw
Copy link
Member Author

dgw commented Nov 22, 2021

New virtual-environments images built today for ubuntu18 and ubuntu20 with Python 3.9.9 listed as one of the changes in release notes. Pre-release status means they're still rolling out, but we're close to having CI again.

@dgw
Copy link
Member Author

dgw commented Nov 25, 2021

Rollout appears to be finished, and our Python 3.9 jobs are passing again.

I will remove this issue from the 7.1.7 release milestone, but leave it open pending activity on python/cpython#29574, an alternative solution to the original problem that bpo-45235 was trying to solve.

@dgw
Copy link
Member Author

dgw commented Jan 18, 2023

Unpinned the issue. There's been no significant activity in the linked issue/PR(s) since my last comment here, and the reason for pinning (Sopel being broken on a current stable Python release) was solved months ago.

@dgw dgw added the Stale Mostly used for PRs that no longer work and need updating before re-review/merge. label Oct 2, 2023
@dgw
Copy link
Member Author

dgw commented Oct 2, 2023

Still no hints of this demon coming back to bite us any time soon. Let's close the issue, trusting our future selves to search for argparse and reopen or reference this issue if it does return in the future.

@dgw dgw closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues High Priority Stale Mostly used for PRs that no longer work and need updating before re-review/merge. Tracking
Projects
None yet
Development

No branches or pull requests

2 participants