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

Ash.Type.File support #214

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

maennchen
Copy link
Contributor

@maennchen maennchen commented Jul 26, 2024

Continuation of ash-project/ash#1337
Based on ash main, needs a release to be merged.

Features

  • File can be provided as Base64 string in JSON body
  • File can be sent as part of a multipart request and referenced in JSON body

Issues

  • Swagger UI can't deal with additionalProperties in multipart
  • Swagger UI can't deal with content type that is not exactly multipart/form-data
  • Support is probably not great in generated clients for multipart

Future tasks

  • Igniter add parser to endpoint on setup

Contributor checklist

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

@maennchen maennchen marked this pull request as draft July 26, 2024 19:03
@maennchen maennchen force-pushed the file_type_support branch 4 times, most recently from 02f691e to 3df5554 Compare July 26, 2024 20:06
@maennchen maennchen marked this pull request as ready for review July 26, 2024 20:09
@zachdaniel
Copy link
Contributor

Very excited about this, will take a look on Monday 🙇

@zachdaniel
Copy link
Contributor

I think we can merge this even w/ the current issues that you've pointed out. I'm thinking the issues laid out would be things we could go forward on as opposed to requiring reworking. For example:

1 & 2. SwaggerUI - the only way forward I can think of here is for this to be resolved on swagger UIs end. We can't accept everything as multipart/form-data
3. we'll have to cross that bridge when we come to it.

Had one comment for discussion, and we can merge once that is settled.

@maennchen maennchen force-pushed the file_type_support branch 2 times, most recently from 926e3de to 9092cac Compare July 29, 2024 14:58
@zachdaniel
Copy link
Contributor

zachdaniel commented Jul 29, 2024

Alright, I think we're good to merge, and we'll roll forward on the swagger UI/codegen issues as people need to leverage these things and/or run into issues. Sound good @maennchen ?

@maennchen
Copy link
Contributor Author

@zachdaniel Agreed 👍

@zachdaniel zachdaniel merged commit beaa320 into ash-project:main Jul 29, 2024
15 checks passed
@maennchen maennchen deleted the file_type_support branch July 29, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants