adafruit / Adafruit_CircuitPython_Requests

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

Importing versions >=1.10.3 broken in CPython #93

Closed kevincon closed 2 years ago

kevincon commented 2 years ago

Some of the imports introduced in #87 (released in version 1.10.3) appear to break the import of adafruit_requests in CPython due to those new imports not being part of the adafruit_requests package's requirements.

To reproduce, you can create a fresh virtual environment and install adafruit-circuitpython-requests==1.10.4:

kevin@penguin:~$ python3 -m venv repro_venv
kevin@penguin:~$ . ./repro_venv/bin/activate
(repro_venv) kevin@penguin:~$ pip install adafruit-circuitpython-requests==1.10.4
Collecting adafruit-circuitpython-requests==1.10.4
  Downloading adafruit-circuitpython-requests-1.10.4.tar.gz (39 kB)
Collecting Adafruit-Blinka
  Downloading Adafruit-Blinka-6.17.0.tar.gz (153 kB)
     |████████████████████████████████| 153 kB 2.8 MB/s 
Collecting Adafruit-PlatformDetect>=3.13.0
  Downloading Adafruit-PlatformDetect-3.18.0.tar.gz (31 kB)
Collecting Adafruit-PureIO>=1.1.7
  Downloading Adafruit_PureIO-1.1.9.tar.gz (26 kB)
Collecting pyftdi>=0.40.0
  Downloading pyftdi-0.53.3-py3-none-any.whl (141 kB)
     |████████████████████████████████| 141 kB 13.6 MB/s 
Collecting pyserial>=3.0
  Downloading pyserial-3.5-py2.py3-none-any.whl (90 kB)
     |████████████████████████████████| 90 kB 3.1 MB/s 
Collecting pyusb!=1.2.0,>=1.0.0
  Downloading pyusb-1.2.1-py3-none-any.whl (58 kB)
     |████████████████████████████████| 58 kB 1.5 MB/s 
Using legacy 'setup.py install' for adafruit-circuitpython-requests, since package 'wheel' is not installed.
Using legacy 'setup.py install' for Adafruit-Blinka, since package 'wheel' is not installed.
Using legacy 'setup.py install' for Adafruit-PlatformDetect, since package 'wheel' is not installed.
Using legacy 'setup.py install' for Adafruit-PureIO, since package 'wheel' is not installed.
Installing collected packages: pyusb, pyserial, pyftdi, Adafruit-PureIO, Adafruit-PlatformDetect, Adafruit-Blinka, adafruit-circuitpython-requests
    Running setup.py install for Adafruit-PureIO ... done
    Running setup.py install for Adafruit-PlatformDetect ... done
    Running setup.py install for Adafruit-Blinka ... done
    Running setup.py install for adafruit-circuitpython-requests ... done
Successfully installed Adafruit-Blinka-6.17.0 Adafruit-PlatformDetect-3.18.0 Adafruit-PureIO-1.1.9 adafruit-circuitpython-requests-1.10.4 pyftdi-0.53.3 pyserial-3.5 pyusb-1.2.1

And then try importing adafruit_requests in the interpreter:

(repro_venv) kevin@penguin:~$ python
Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import adafruit_requests
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/kevin/repro_venv/lib/python3.9/site-packages/adafruit_requests.py", line 107, in <module>
    class Response:
  File "/home/kevin/repro_venv/lib/python3.9/site-packages/adafruit_requests.py", line 114, in Response
    def __init__(self, sock: SocketType, session: Optional["Session"] = None) -> None:
NameError: name 'SocketType' is not defined

I believe one of the imports of adafruit_esp32spi, adafruit_wiznet5k, etc. in this try block fail with ImportError since they are not requirements of adafruit-circuitpython-requests and thus don't get installed by pip, so the SocketType type var does not get defined (hence the error above):

https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/26b2411780f69edece156c25476359242446bac0/adafruit_requests.py#L41-L68

but since the try block's exception handler passes on ImportError, we don't see an error until the first attempted usage of SocketType in this type annotation:

https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/26b2411780f69edece156c25476359242446bac0/adafruit_requests.py#L114

I read on Discord someone else ran into this as well: https://discord.com/channels/327254708534116352/537365702651150357/925282376525959229

Since those adafruit_esp32spi, adafruit_wiznet5k, etc. packages don't seem to be used beyond defining the SocketType, InterfaceType, etc. TypeVars, could protocols be used to define the shape of those types to avoid needing to import those packages?

makermelissa commented 2 years ago

I'm fairly sure CPython includes its own requests library which works well. Is this a dependency of some library you are trying to use?

kevincon commented 2 years ago

My use case is that I'm developing a simulator for CircuitPython projects that runs project code in CPython (similar to Device Simulator Express but without being coupled to VSCode and having an additional goal to enable automated testing of CircuitPython projects in the simulator). The first board this simulator supports is the MagTag, and the Adafruit_CircuitPython_MagTag library which many MagTag projects use depends on adafruit-circuitpython-requests, so I currently can't run most MagTag projects in the simulator using the latest version of adafruit-circuitpython-requests while it can't be imported and executed under CPython.

But I think this issue also impacts any CircuitPython projects which depend on adafruit-circuitpython-requests and use Adafruit Blinka to support running their code under CPython on Raspberry Pi and other Linux boards, e.g. see https://github.com/search?q=adafruit-circuitpython-requests+filename%3Arequirements.txt+language%3AText+path%3A%2F+filename%3Arequirements.txt&type=Code&ref=advsearch&l=&l=.

FoamyGuy commented 2 years ago

That project you are working on @kevincon sounds awesome!

Blinka projects on RasPi or similar devices that are running CPython would have access to the "standard" python requests library, and they'd likely benefit from using that instead of the CircuitPython one which is designed as a subset of the standard one.

I think It would be good to get it back to being able to imported on CPython nonetheless though because some of the files in this repo in tests/ and examples/ seem to be made to be run on CPython. If I understand this issue correctly those would not be working ATM. I can definitely see the benefit of having this work as identically as possible in CPython for automated testing and such.

I am still a bit new to the various ways to use typing in python. It does look like your proposed idea to use protocols could be a good solution to me. If you're interested in submitting a PR that would be awesome. If not I'll add this to my list and circle back to it at some point to attempt that change. In the meantime installing those extra libraries (and modifying requirements.txt if you'd like) are probably the easiest things to do to get it working.

makermelissa commented 2 years ago

Awesome. Perhaps you can figure out some of the issues that affect the CircuitPython library.

kevincon commented 2 years ago

Thanks @FoamyGuy, while I agree that projects that aim to only run on Raspberry Pi and similar devices can use requests instead of adafruit-circuitpython-requests, I'm more thinking of projects and libraries that aim to be simultaneously compatible with both CPython (via Blinka) as well as CircuitPython, e.g. https://github.com/adafruit/Adafruit_CircuitPython_OAuth2 and https://github.com/adafruit/Adafruit_CircuitPython_PortalBase seem to want to do that and depend on adafruit-circuitpython-requests. I guess you could argue that projects like those should only depend on adafruit-circuitpython-requests for CircuitPython and separately depend on requests for CPython, but I can appreciate that depending on a single library (adafruit-circuitpython-requests) may make them simpler to maintain.

I'll work on making a PR to fix this and also try to incorporate importing adafruit-circuitpython-requests somehow in the Build CI job to help prevent this from breaking again.