-
Notifications
You must be signed in to change notification settings - Fork 119
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
fix(pebble): remove use of deprecated cgi module in Pebble code #996
Conversation
The recommendation is to use the email.message.Message type. See example: https://docs.python.org/3.11/library/cgi.html#cgi.parse_header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor nits, but this looks good! It feels a little weird to me that we are using the email module to interface with a service manager, but I'm glad that it was not too much work to replace the cgi
module 😅
with self.assertRaises(pebble.ProtocolError) as cm: | ||
self.client.pull('/etc/hosts') | ||
self.assertIsInstance(cm.exception, pebble.Error) | ||
self.assertEqual(str(cm.exception), | ||
"expected Content-Type 'multipart/form-data', got 'ct'") | ||
"expected Content-Type 'multipart/form-data', got 'c/t'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why you needed to change this from ct
to c/t
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the test started failing without it, because the new email.message.Message
header parsing validates that the content type is of the form type/subtype
.
@NucciTheBoss No kidding, feels weird to me too! It'd be much saner if it was |
The recommendation is to use the email.message.Message type. See example: https://docs.python.org/3.11/library/cgi.html#cgi.parse_header
Fixes #995