adafruit/Adafruit_CircuitPython_Requests

Importing versions >=1.10.3 broken in CPython

kevincon opened this issue · 5 comments

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):

try:
from typing import Union, TypeVar, Optional, Dict, Any, List, Type
import types
from types import TracebackType
import ssl
import adafruit_esp32spi.adafruit_esp32spi_socket as esp32_socket
import adafruit_wiznet5k.adafruit_wiznet5k_socket as wiznet_socket
import adafruit_fona.adafruit_fona_socket as cellular_socket
from adafruit_esp32spi.adafruit_esp32spi import ESP_SPIcontrol
from adafruit_wiznet5k.adafruit_wiznet5k import WIZNET5K
from adafruit_fona.adafruit_fona import FONA
import socket as cpython_socket
SocketType = TypeVar(
"SocketType",
esp32_socket.socket,
wiznet_socket.socket,
cellular_socket.socket,
cpython_socket.socket,
)
SocketpoolModuleType = types.ModuleType
SSLContextType = (
ssl.SSLContext
) # Can use either CircuitPython or CPython ssl module
InterfaceType = TypeVar("InterfaceType", ESP_SPIcontrol, WIZNET5K, FONA)
except ImportError:
pass

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:

def __init__(self, sock: SocketType, session: Optional["Session"] = None) -> None:

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?

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?

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=.

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.

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

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.