Skip to content

Importing versions >=1.10.3 broken in CPython #93

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.

Sign up for GitHub

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

Jump to bottom
Closed
kevincon opened this issue Jan 1, 2022 · 5 comments · Fixed by #94
Closed

Importing versions >=1.10.3 broken in CPython #93

kevincon opened this issue Jan 1, 2022 · 5 comments · Fixed by #94

Comments

@kevincon
Copy link
Contributor

kevincon commented Jan 1, 2022

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

Adafruit_CircuitPython_Requests/adafruit_requests.py

Lines 41 to 68 in 26b2411

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:

Adafruit_CircuitPython_Requests/adafruit_requests.py

Line 114 in 26b2411

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?
@makermelissa
Copy link
Collaborator

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
Copy link
Contributor Author

kevincon commented Jan 3, 2022

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
Copy link
Contributor

FoamyGuy commented Jan 3, 2022

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
Copy link
Collaborator

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

@kevincon
Copy link
Contributor Author

kevincon commented Jan 3, 2022

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.

@kevincon kevincon mentioned this issue Jan 6, 2022
Merged
@makermelissa makermelissa mentioned this issue Jan 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix importing adafruit_requests in CPython kevincon/Adafruit_CircuitPython_Requests
3 participants
@kevincon @FoamyGuy @makermelissa