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

remove backwards compatibility #135

Merged
merged 4 commits into from
Jul 8, 2023

Conversation

FoamyGuy
Copy link
Contributor

This change would go along with: adafruit/Adafruit_CircuitPython_ESP32SPI#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
Copy link
Contributor Author

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 adafruit/Adafruit_CircuitPython_ESP32SPI#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
Copy link
Contributor Author

FoamyGuy commented Jun 26, 2023

The latest commit removes the legacy tests which were validating behavior related to the non-standard socket behavior from ESP32SPI. adafruit/Adafruit_CircuitPython_ESP32SPI#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 adafruit/Adafruit_CircuitPython_ESP32SPI#167 so that the released versions in the bundle stay in sync and compatible with each other.

@FoamyGuy FoamyGuy requested a review from a team June 26, 2023 21:47
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you! Not merging so you can time it right.

adafruit_requests.py Outdated Show resolved Hide resolved
@FoamyGuy FoamyGuy merged commit e09c3bd into adafruit:main Jul 8, 2023
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jul 9, 2023
Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 6.0.0 from 5.0.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#167 from dhalbert/compatible-socket

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 7.4.0 from 7.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#169 from vladak/loop_vs_timeout
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#162 from adafruit/settings_dot_toml
  > Update .pylintrc, fix jQuery for docs
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#167 from tekktrik/main
  > Run pre-commit
  > Update pre-commit hooks
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#164 from zbauman3/zane/cert-key-pair-example

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 2.0.0 from 1.14.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#135 from FoamyGuy/remove_backward_compat

Updating https://github.com/adafruit/Adafruit_CircuitPython_WSGI to 2.0.0 from 1.1.16:
  > Merge pull request adafruit/Adafruit_CircuitPython_WSGI#20 from FoamyGuy/compatibility_refactor
  > Merge pull request adafruit/Adafruit_CircuitPython_WSGI#19 from tekktrik/dev/fix-packaging

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
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