Skip to content

Support python 3.11 #241

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 25, 2023
Merged

Support python 3.11 #241

merged 7 commits into from
May 25, 2023

Conversation

apotterri
Copy link
Contributor

@apotterri apotterri commented May 18, 2023 •
edited
Loading

Please don't merge this, it needs a rebase

Fixes #236.

@apotterri apotterri requested a review from dividedmind May 18, 2023 22:35
dividedmind
dividedmind requested changes May 19, 2023
View reviewed changes
Copy link
Contributor

@dividedmind dividedmind left a comment

Choose a reason for hiding this comment

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

Judging by the CI log, it seems that moving wrapt to _appmap might not have been the best idea. What is the reason to move, how was it more difficult to upgrade when it was in appmap? The commit message doesn't make it entirely clear.

Other than that and the comments, please add 3.11 to CI (and maybe move smoke test to that).

@apotterri apotterri force-pushed the python-3.11_20230515 branch 3 times, most recently from 7910981 to d525b84 Compare May 22, 2023 21:07
apotterri
apotterri commented May 22, 2023
View reviewed changes
_appmap/wrapt
@@ -0,0 +1 @@
../vendor/_appmap/wrapt No newline at end of file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is here to make development easier. It's exclude from the wheel when it gets built: https://github.com/getappmap/appmap-python/pull/241/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R25

@apotterri apotterri force-pushed the python-3.11_20230515 branch from d525b84 to 1b05a08 Compare May 22, 2023 22:07
@apotterri
Copy link
Contributor Author

@dividedmind this is ready for another look

I made one fairly substantial change since you last looked: I removed the wrapt submodule, and replaced it by the use of the vendoring package. This seems to be a more widely-used approach to vendoring. One of its benefits is that it will allow us to easily vendor more packages, which we may need sooner rather than later: https://appmap-group.slack.com/archives/C03GM7D2QRH/p1684786853793949?thread_ts=1684779111.885699&cid=C03GM7D2QRH

@dustinbyrne dustinbyrne requested a review from dividedmind May 23, 2023 14:04
dividedmind
dividedmind reviewed May 24, 2023
View reviewed changes
dividedmind
dividedmind approved these changes May 24, 2023
View reviewed changes
Copy link
Contributor

@dividedmind dividedmind left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the changes! (I think I liked submoduling more, but if vendoring is easier to work with, that's fine.)

apotterri added 7 commits May 25, 2023 05:24
@apotterri
refactor: undo rename of wrapt
2776194 
Renaming wrapt to appmap.wrapt has made it prohibitively difficult to
update it, and wasn't really necessary. Instead, just symlink it under
_appmap and import it appropriately.
@apotterri
refactor: switch to vendoring
646d1bc 
Use the vendoring package to install and patch v1.15.0 of wrapt.
@apotterri
fix: handle 3.11 find_spec implementation
c62e64e 
3.11 changed some aspect of how finders are implemented, and broke our
previous approach to wrapping find_spec. These changes make wrapping
work again.
@apotterri
refactor: split appmap.unittest
e00527a 
Move the implementation details of unittest integration into
_appmap.unittest.
@apotterri
fix: update unittest integration for python 3.11
bd9598b 
3.11 made updates to the implementation of unittest which broke our
integration. These changes make it work again for all supported
versions.
@apotterri
feat: support python 3.11
15b0ddc 
Update the test matrix to include 3.11.
@apotterri
fix: relax restriction on packaging to >=19.0
affdbda 
23.1 was the current version when we first added packaging, which was
why it was chosen.

We don't really need anything except conformance to PEP 508, though.
19.0 was the last time packaging's changelog mentions that PEP, so
that should be good enough.
@apotterri apotterri force-pushed the python-3.11_20230515 branch from 0481d2d to affdbda Compare May 25, 2023 09:24
@apotterri apotterri merged commit cdefb2d into master May 25, 2023
@apotterri apotterri deleted the python-3.11_20230515 branch November 4, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dividedmind dividedmind dividedmind approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Python 3.11 not supported due to vendored wrapt

2 participants

@apotterri @dividedmind