Skip to content

Conversation

@amwell-engineering
Copy link
Owner

Replaces use of cgi

# Adapted from https://peps.python.org/pep-0594/#cgi
m = Message()
m['content-type'] = value
return dict(m.get_params())
Copy link

@mayoroftuesday mayoroftuesday Jan 15, 2025

Choose a reason for hiding this comment

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

This doesn't seem quite right. It returns a single value, but the return type is expected to be two values. Also the get_params method is stated to be a legacy function. I think we should be using the EmailMessage replacement described here: https://docs.python.org/3.12/library/cgi.html#cgi.parse_header

def parse_separated_header(value: str):
    msg = EmailMessage()
    msg['content-type'] = 'application/json; charset="utf8"'
    main, params = msg.get_content_type(), msg['content-type'].params
    return main, params

docs/urls.html Outdated
if not content_type:
return default
content_type, params = parse_header(content_type)
content_type, params = parse_separated_header(content_type)

Choose a reason for hiding this comment

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

This documentation does not match the code. Is this auto-generated, or do we have to edit this by hand?

Should be:

content_type = Parse_separated_header(content_type)

Choose a reason for hiding this comment

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

There are other changes not present in this documentation file as well -- the parse_separated_header function, the imports... This should be generated by pdoc3, but I think the original author gitignored any files related to generating docs, so it's hard to know what their process was. If I just run pdoc on this project, I get slightly broken formatting, but it looks close to the original docs.

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.

3 participants