adafruit / Adafruit_CircuitPython_Requests

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

remove backwards compatibility #135

Closed FoamyGuy closed 1 year ago

FoamyGuy commented 1 year ago

This change would go along with: https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/167. They would need to be merged at the same time to keep things working I think.

If that ESP32SPI PR is merged it will make the sockets share the same API so I think this backwards compatibility behavior is no longer needed.

The FakeSSLSocket does need to have a recv_into binding in order for it to actually work with this removed.

Using the changes from this PR and the changes from ESP32SPI PR together on a PyPortal I am able to successfully wget() the image file without any stair stepping chunked issues.

FoamyGuy commented 1 year ago

I'm not entirely sure how to re-create the failed test that actions is failing for locally. I tried running the test locally but it seems to pass: image

It seems like the tests failing are for legacy compatible behavior. If the behavior that is referring to is the stuff being removed by https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/167 then perhaps we don't need to keep the tests for it since the socket APIs will match after that if I understand correctly.

If we do need to keep the legacy tests, it looks like it will need to have recv_into() added to the mocket in legacy_modcket.py. Similar to how it was added to FakeSSLContext I guess. But I'm not really sure how to implement it in the mocket, I could create a function for it, but I don't know what would need to go inside of it.

FoamyGuy commented 1 year ago

The latest commit removes the legacy tests which were validating behavior related to the non-standard socket behavior from ESP32SPI. https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/167 will make the socket API more standardized and matching with CPython, so these tests are no longer needed.

I think this is ready for review.

I tested these changes successfully in the following contexts:

Ideally this should get merged / released at the same time as https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/167 so that the released versions in the bundle stay in sync and compatible with each other.