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

Add/multipart form data test #198

Closed
wants to merge 2 commits into from

Conversation

hsundell
Copy link

Based on #195

I added also a test for multipart/form-data which seems to fail at the moment.

I encountered this issue when updating a project that had ancient requirements but worked with those.

pyramid-openapi3 <= 0.14.0
openapi-core <= 0.16.7
openapi-schema-validator <= 0.2.3
openapi-spec-validator <= 0.4.0

I don't know exactly what has broken and where, but definitely openapi-core 0.16.8 changed something.

With these versions the form data was processed. Now it fails to deserialize unless I add a custom deserializer which turns the data into json-serializable dict. I wouldn't want to do this for several reasons:

  1. Unnecessarily complicated, should work out-of-the-box
  2. Requires custom handling of more complicated data types
  3. File data in the forms which turn into cgi.FieldStorage objects used to contain streamable BytesIO objects which could be nicely processed in business logic in small chunks. With custom deserializaer I had to convert it into string before OpenAPI validation would pass. Maybe there'd be a better workaround but I'd of course prefer to get the issue fixed instead.

@zupo
Copy link
Collaborator

zupo commented Mar 7, 2024

Hey @hsundell, support for the latest openapi-core 0.19 was just merged! Could you please rebase this PR off of latest main so we see what's the latest status with multipart form data? Even if tests now magically pass, I'd still like to add some tests so we catch any regressions in the future.

@am-on
Copy link
Contributor

am-on commented Mar 12, 2024

I think this pull request can be closed since #225 was merged.

@zupo zupo closed this Mar 12, 2024
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