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

Replace deprecated cgi module usage with email #340

Merged
merged 2 commits into from Mar 14, 2023

Conversation

mtreinish
Copy link
Contributor

In Python 3.11 the standard library cgi module was deprecated with a planned removal set for Python 3.13. In preparation for that removal, this commit removes the usage of this deprecated module and replaces it with the still supported standard library email module which is what the documentation points to as an alternative for how cgi was previously used. This should still be compatible with all the supported Python versions but will be more future proof and not emit any deprecation warnings with Python 3.11 anymore.

In Python 3.11 the standard library `cgi` module was deprecated with a
planned removal set for Python 3.13. In preparation for that removal,
this commit removes the usage of this deprecated module and replaces it
with the still supported standard library `email` module which is what
the documentation points to as an alternative for how `cgi` was
previously used. This should still be compatible with all the supported
Python versions but will be more future proof and not emit any
deprecation warnings with Python 3.11 anymore.
Copy link
Contributor

@jugmac00 jugmac00 left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request. There is one change necessary, then I think we are good to go.

msg = email.message.EmailMessage()
msg['content-type'] = mime_type

full_type, parameters = msg.get_content_type(), msg['content-type'].params
Copy link
Contributor

Choose a reason for hiding this comment

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

The new parameters now is a mappingproxy, so your solution is not equivalent to the old one.

When you scroll down a bit you can see that parameters will be manipulated, which does not work for a mappingproxy.

Unfortunately, the test coverage does not seem to be high for this package, so this was not uncovered.

You can workaround that issue when you wrap msg['content-type'].params in a dict().

Copy link
Contributor

Choose a reason for hiding this comment

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

@mtreinish Do you think you can push the requested changes? I see the deprecation warning daily and I'd really like to get rid of it :-D Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay I missed this comment at first. I've added the dict wrapping in: 54ea8f2

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.

None yet

3 participants