Skip to content

Conversation

@daanvdk
Copy link
Contributor

@daanvdk daanvdk commented Apr 1, 2021

Adds a possibility to send files deeply nested within data.

To do this you have to send a multipart/form-data request with the data encoded as json in a field called data. The files within this data have to be replaced with null and added to the multipart data as a file, the name of this file should be file:<path> where <path> is the keys that lead to the file joined with ..

Some other notable changes: So because of this I wanted to load json that is a str instead of only bytes, so I removed the .decode() in jsonloads. This is backwards compatible for python >=3.6 since in that release json.loads was changed to also accept binary data.

@daanvdk daanvdk force-pushed the files-in-request-body branch from 87d8fe7 to 00b4b2c Compare April 9, 2021 14:00
Copy link

@stefanmajoor stefanmajoor left a comment

Choose a reason for hiding this comment

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

The idea is good, and it is getting there. I do miss two things:

  • Documentation. Both in the code (describe what new methods do, inline comments) as well as general documentation (this is how the method works). I had to try to figure out what exactly you were trying to implement from the implementation
  • Only the good weather scenario is tested for the file upload. Can you also add tests for the edge cases? e.g. what happens if you do not set the "Data", or the "data" is incorrect. O

@daanvdk daanvdk requested a review from stefanmajoor July 5, 2021 13:53
@daanvdk daanvdk force-pushed the files-in-request-body branch from 99ded81 to 790d075 Compare September 7, 2021 09:36
@daanvdk daanvdk requested a review from stefanmajoor September 7, 2021 09:43
When a model has annotations, it will crash. Also scope on pk.

TODO: NEEDS TEST!
@daanvdk daanvdk merged commit 55c8076 into master Sep 28, 2021
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