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

email parse depends on python patch version #1146

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

austinweisgrau
Copy link
Collaborator

The behavior of email.parseaddr depends on the python patch version.

See python/cpython#102988 or associated changelogs, e.g. https://docs.python.org/3.8/whatsnew/changelog.html#python-3-8-20-final

@austinweisgrau austinweisgrau mentioned this pull request Oct 9, 2024
3 tasks
@bmos
Copy link
Contributor

bmos commented Oct 9, 2024

Great work tracking down the changelog that describes this behavior!

Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

Thanks so much for tracking this down! It was a tricky one. I like your solution to add a condition to the tests, rather than require folks to not pin to a specific python version.

@shaunagm shaunagm merged commit 22c3175 into move-coop:main Oct 10, 2024
28 checks passed
@shaunagm
Copy link
Collaborator

Although, question: how would this create the pattern of test failures we saw, where tests were failing on Mac and Windows up to 3.12 (but not 3.12) and Ubuntu on 3.8 (but not 3.9-3.12)?

@shaunagm shaunagm added the breaking change applied only to PRs, indicates when something has a breaking change to be flagged in release notes label Oct 10, 2024
@austinweisgrau
Copy link
Collaborator Author

austinweisgrau commented Oct 10, 2024

There isn't actually a breaking change in this PR, rather the change in behavior of email.parseaddr is with the python minor patch version which is very upstream of us.

I believe the different test failures were due to different times that the patches were merged into different python versions. The change in parseaddr was merged into each python major version in different releases at presumably different times.

@shaunagm
Copy link
Collaborator

Sorry, that was a bit of inside baseball - I'm cutting the next release soon and I always check the PRs for breaking changes. When I see this PR, I'll remember to add a small note to the release notes calling out this fix in particular, just in case it's affected anyone else. We had a hard enough time detecting that it was due to a python patch version, so if a Parsons user runs into it I'd hope they could see it in the Parsons changelog notes instead of having to delve even further.

tl;dr I won't actually list it as a breaking change.

@shaunagm
Copy link
Collaborator

I believe the different test failures were due to different times that the patches were merged into different python versions. The change in parseaddr was merged into each python major version in different releases at presumably different times.

Also this explanation makes sense, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change applied only to PRs, indicates when something has a breaking change to be flagged in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants