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

Support all IANA mime types #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dkoston
Copy link

@dkoston dkoston commented Dec 9, 2023

Overview

We have some content types in our schemas that were not supported. Instead of just adding the few we needed, I decided to grab all the IANA types as CSVs and build a tool to output the ContentType enum. I moved it out to mime_types.py as that's a generated file so, it should be kept separate.

NOTE: this should be merged after the switch to poetry PR as it is branched off that branch

Changes

build_mime_types: Add tool to create ContentType enum from all IANA mime type CSVS
pyproject.toml: Run build_mime_types.py during poetry build
src/tests: Add more tests around mime types and adjust to mime_types.ContentType from enumeration.ContentType

@@ -0,0 +1,9 @@
# build-mime-types

Builds a class file for ContentType enum from the IANA mime types list.
Copy link
Owner

Choose a reason for hiding this comment

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

I really like the approach with code-generation based on the standards, but since there is no automation to get the latest version of those csv files - it makes the module useful just for one-time execution. I think there is no need to include it into the repository since the process of extending will be identical (update csv of the actual python enum file)

Copy link
Author

@dkoston dkoston Dec 11, 2023

Choose a reason for hiding this comment

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

Where would the code live if it doesn't live in this repo?

since the process of extending will be identical (update csv of the actual python enum file)

Process:

  1. Download CSV files
  2. Run build_mime_types.py

If you delete build_mime_types.py you'll have to add thousands of enum lines by hand or do a manual diff. Maybe I'm missing something?

Copy link
Owner

Choose a reason for hiding this comment

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

I agree with this approach, but for now it's semi-automated. I meant that if we go with code generation, then we can exclude CSV files from the git and make them downloadable directly in the script to have all the latest versions w/o manual actions

Copy link
Owner

@manchenkoff manchenkoff Dec 11, 2023

Choose a reason for hiding this comment

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

In this case your suggestion with enum map for existing values would work fine as well, so it is up to you: update it now or leave PR as is and wait till the major version update w/o BC (but I have no time estimate on that for now)

Copy link
Author

Choose a reason for hiding this comment

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

I added the maps. There are some open api specific content-types not listed in IANA that I added / generated and some that this library names specifically so I added logic for those to keep everything backwards compatible.

src/openapi_parser/mime_types.py Outdated Show resolved Hide resolved
tests/builders/test_content_builder.py Outdated Show resolved Hide resolved
tests/test_enumeration.py Show resolved Hide resolved
build_mime_types: Add tool to create ContentType enum from all IANA mime type CSVS
pyproject.toml: Run build_mime_types.py during `poetry build`
src/tests: Add more tests around mime types and adjust to mime_types.ContentType from enumeration.ContentType
"multipart": "MULTIPART_ANY",
"text/*": "TEXT_ANY",
"video/*": "VIDEO_ANY",
'text/json': "JSON_TEXT", # This isn't an OpenAPI specific mime type but it is needed as it already is in use
Copy link
Author

Choose a reason for hiding this comment

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

FWIW: This content type is deprecated. We may want to mark as such so it can be removed later?

https://mimetype.io/text/json

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good to me 👍

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.

2 participants