search

adafruit / Adafruit_CircuitPython_Requests

Requests-like interface for web interfacing
MIT License
51 stars 37 forks source link

Files arg #189

Closed FoamyGuy closed 4 months ago

FoamyGuy commented 5 months ago

This change adds a files argument that can be passed to post()

The value supported is dictionary with string keys and tuple values. The behavior when passed matches the behavior of CPython requests.

A new example was added illustrating the functionality, I've only tested with native wifi on ESP32 S3, so that is the only one added.

I was able to confirm the upload is working correctly by using a local simple flask server that acceps file upload and by examining the result returned by httpbin.org when the new example runs. It returns the image back in base64 format. This cyberchef recipe was used to validate that the original image does pop back out the other side https://cyberchef.org/#recipe=From_Base64('A-Za-z0-9%2B/%3D',true,false)Render_Image('Raw')

Note that the CPYthon implementation of this also supports a files dictionary with string keys and file_handle values (as opposed to tupple containing the file_handle and other things). That form of the argument is not supported on circuitpython. In order for that to be supported I believe we would need access to FileIO.name property or something that provides similar functionality. I've created #9195 in the core inquiring about adding that, but at present time this is not supported in the circuitpython version.

dhalbert commented 5 months ago

@FoamyGuy is this ready for final review or is it still in progress/

FoamyGuy commented 5 months ago

I believe this is ready for review now. I've merged Justin's changes that were submitted against my branch to this so they are included here now.

justmobilize commented 5 months ago

@dhalbert I created a new PR into his branch to add some tests back in. Would love to see those in (can add later if wanted)

justmobilize commented 5 months ago

@dhalbert my tests are merged and this is ready for review!

justmobilize commented 5 months ago

@FoamyGuy shall I make another PR into this branch with the changes?

FoamyGuy commented 5 months ago

@FoamyGuy shall I make another PR into this branch with the changes?

I got the change to os.urandom() with the latest commit, if you've got a clear idea how to tackle reducing the string concatenation then definitely feel free to PR to this branch.

I'll try to take a crack at it over the weekend if you don't have the opportunity to get it first.

justmobilize commented 5 months ago

@FoamyGuy created here: https://github.com/FoamyGuy/Adafruit_CircuitPython_Requests/pull/4

justmobilize commented 5 months ago

@dhalbert ready to go again!

dhalbert commented 4 months ago

Maybe the ValueError -> AttributeError should be a separate PR.

justmobilize commented 4 months ago

@dhalbert I'm happy to work through this repo and fix up the exceptions and tests, and can do so after this is merged if that is the only concern.

justmobilize commented 4 months ago

@dhalbert I had this idea of a requests-plus. That requires normal requests but has extra methods and overrides. The other option is that requests-plus could just have add-in methods (like in this case _build_boundary_string, _build_boundary_data etc. And if you pass in files and we got an import error for requests-plus it would error...

dhalbert commented 4 months ago

@FoamyGuy Thanks for merging. This is an upward-compatible improvement, so it deserved a minor version bump. But no need to change the version number now.