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

Python 3.13 support #302

Merged
merged 13 commits into from
Dec 12, 2024
Merged

Python 3.13 support #302

merged 13 commits into from
Dec 12, 2024

Conversation

ljstella
Copy link
Contributor

@ljstella ljstella commented Oct 7, 2024

Python 3.13.0 was released today: https://www.python.org/downloads/release/python-3130/

I do not think there's any python 3.13.0 specific features we want or need, but I would like to test to confirm contentctl functions as expected with 3.13

@ljstella
Copy link
Contributor Author

ljstella commented Oct 7, 2024

Current status: I think we're waiting on Python 3.13 to land in actions/python-versions so that actions/setup-python can use it.

This also pushes us up close to 50 jobs- which is a lot! I think we can pare this back a bit too- at the moment, there are no macOS 15 runners, so macos-14 and macos-latest are duplicates currently. Additionally, I think we have some duplication between pull_request and push that we can trim.

@ljstella
Copy link
Contributor Author

ljstella commented Oct 8, 2024

Update: Waiting on actions/python-versions#312 to merge

@ljstella
Copy link
Contributor Author

ljstella commented Oct 8, 2024

Update: python-version and setup-python are working now, python 3.13 installs.

Currently blocked on: pygit2
Details: libgit2/pygit2#1317

Looks like pygit2 on master currently builds with 3.13, but there isn't a release yet.

@ljstella
Copy link
Contributor Author

ljstella commented Oct 8, 2024

Tweaked the matrices and OS:

  • macos-15 is in beta, so its not being picked up by macos-latest. Tweaked macos-latest to macos-15 so we can explicitly test against it.

  • Dropped ubuntu-22.04 in favor of testing against ubuntu-20.04 (old release) and adding ubuntu-24.04 (new release). We have very few system specific dependencies since we don't use the system python, so in the future, I would not be opposed to reducing this to one option.

  • Removed on: push: from the trigger conditions and added synchronize to the list of pull_request types- this reduces the total number of jobs spawned on PRs

Edit: With these changes to the tests being run, the rules for expected checks need to be revisited.

@ljstella
Copy link
Contributor Author

pygit2 has been updated and contentctl now installs. Looks like there's a bit of an issue with Tyro now though:

/Users/lstella/ThreatResearch/github/contentctl/.venv/lib/python3.13/site-packages/splunklib/client.py:772: SyntaxWarning: invalid escape sequence '\/'
  versionSearch = re.search('(?:servicesNS\/[^/]+\/[^/]+|services)\/[^/]+\/v(\d+)\/', path)
Traceback (most recent call last):
  File "/Users/lstella/ThreatResearch/github/contentctl/.venv/bin/contentctl", line 6, in <module>
    sys.exit(main())
             ~~~~^^
  File "/Users/lstella/ThreatResearch/github/contentctl/contentctl/contentctl.py", line 166, in main
    models = tyro.extras.subcommand_type_from_defaults(
        {
    ...<10 lines>...
        }
    )
  File "/Users/lstella/ThreatResearch/github/contentctl/.venv/lib/python3.13/site-packages/tyro/extras/_base_configs.py", line 134, in subcommand_type_from_defaults
    tuple(
    ~~~~~^
        Annotated.__class_getitem__(  # type: ignore
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<10 lines>...
        for k, v in defaults.items()
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/lstella/ThreatResearch/github/contentctl/.venv/lib/python3.13/site-packages/tyro/extras/_base_configs.py", line 135, in <genexpr>
    Annotated.__class_getitem__(  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/typing.py", line 548, in __getattr__
    raise AttributeError(item)
AttributeError: __class_getitem__

@ljstella
Copy link
Contributor Author

Pending on brentyi/tyro#200 to add Python 3.13 support. Confirmed current error appears to be addressed in the changes.

@ljstella
Copy link
Contributor Author

Tyro 0.9.0 was released (This closes #328) but was yanked due to an issue. Once its re-released, this should be good to go.

@ljstella
Copy link
Contributor Author

Tyro 0.9.1 introduced a whole new issue for us: brentyi/tyro#203

@ljstella
Copy link
Contributor Author

Re: Tyro 0.9.1- current issues are caused by the use_enum_values config for pydantic that we've already decided we want to remove (see #266 )- we should prioritize that work.

@ljstella ljstella marked this pull request as ready for review November 25, 2024 17:22
@ljstella
Copy link
Contributor Author

Marking this as "Ready for Review"

the latest version of Tyro should work for us again. We can either merge this sooner (probably pin the Tyro version too) and get python3.13 support out, or wait on the removal of the pydantic use_enum_value config from the codebase, where we'll be able to more reasonably rely on future versions of Tyro not breaking for us.

@pyth0n1c pyth0n1c changed the base branch from main to contentctl_5 December 4, 2024 19:17
@pyth0n1c pyth0n1c merged commit fde8b9a into contentctl_5 Dec 12, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants