Skip to content

Fix importing adafruit_requests in CPython #94

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

Merged
merged 6 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ jobs:
awk -F '\/' '{ print tolower($2) }' |
tr '_' '-'
)
- name: Set up Python 3.7
- name: Set up Python 3.8
uses: actions/setup-python@v1
with:
python-version: 3.7
python-version: 3.8
- name: Versions
run: |
python3 --version
Expand Down Expand Up @@ -71,5 +71,9 @@ jobs:
python setup.py sdist
python setup.py bdist_wheel --universal
twine check dist/*
- name: Test Python package
run: |
pip install tox==3.24.5
tox
- name: Setup problem matchers
uses: adafruit/circuitpython-action-library-ci-problem-matchers@v1
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
if: contains(steps.need-pypi.outputs.setup-py, 'setup.py')
uses: actions/setup-python@v1
with:
python-version: '3.x'
python-version: '3.8'
- name: Install dependencies
if: contains(steps.need-pypi.outputs.setup-py, 'setup.py')
run: |
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

*.mpy
.idea
.vscode
__pycache__
_build
*.pyc
.env
.tox
bundles
*.DS_Store
.eggs
Expand Down
2 changes: 1 addition & 1 deletion .readthedocs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
version: 2

python:
version: "3.7"
version: "3.8"
install:
- requirements: docs/requirements.txt
- requirements: requirements.txt
145 changes: 113 additions & 32 deletions adafruit_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,116 @@
__repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_Requests.git"

import errno
import sys

if sys.implementation.name == "circuitpython":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this pattern was used to only run the typing-related code in Python implementations which support it (such as CPython):

try:
    # typing-related code here
    ...
except ImportError:
    pass

But that caused the CPython import failure issue described in #93 to be swallowed, so to help make debugging similar issues easier in the future, I replaced that pattern with this approach of reading sys.implementation.name to check if CircuitPython is being used or not instead.


def cast(_t, value):
"""No-op shim for the typing.cast() function which is not available in CircuitPython."""
return value


else:
from ssl import SSLContext
from types import ModuleType, TracebackType
from typing import Any, Dict, List, Optional, Protocol, Tuple, Type, Union, cast

# Based on https://github.com/python/typeshed/blob/master/stdlib/_socket.pyi
class CommonSocketType(Protocol):
"""Describes the common structure every socket type must have."""

def send(self, data: bytes, flags: int = ...) -> None:
"""Send data to the socket. The meaning of the optional flags kwarg is
implementation-specific."""
...

def settimeout(self, value: Optional[float]) -> None:
"""Set a timeout on blocking socket operations."""
...

def close(self) -> None:
"""Close the socket."""
...

class CommonCircuitPythonSocketType(CommonSocketType, Protocol):
"""Describes the common structure every CircuitPython socket type must have."""

def connect(
self,
address: Tuple[str, int],
conntype: Optional[int] = ...,
) -> None:
"""Connect to a remote socket at the provided (host, port) address. The conntype
kwarg optionally may indicate SSL or not, depending on the underlying interface."""
...

class LegacyCircuitPythonSocketType(CommonCircuitPythonSocketType, Protocol):
"""Describes the structure a legacy CircuitPython socket type must have."""

def recv(self, bufsize: int = ...) -> bytes:
"""Receive data from the socket. The return value is a bytes object representing
the data received. The maximum amount of data to be received at once is specified
by bufsize."""
...

class SupportsRecvWithFlags(Protocol):
"""Describes a type that posseses a socket recv() method supporting the flags kwarg."""

def recv(self, bufsize: int = ..., flags: int = ...) -> bytes:
"""Receive data from the socket. The return value is a bytes object representing
the data received. The maximum amount of data to be received at once is specified
by bufsize. The meaning of the optional flags kwarg is implementation-specific."""
...

class SupportsRecvInto(Protocol):
"""Describes a type that possesses a socket recv_into() method."""

def recv_into(
self, buffer: bytearray, nbytes: int = ..., flags: int = ...
) -> int:
"""Receive up to nbytes bytes from the socket, storing the data into the provided
buffer. If nbytes is not specified (or 0), receive up to the size available in the
given buffer. The meaning of the optional flags kwarg is implementation-specific.
Returns the number of bytes received."""
...

class CircuitPythonSocketType(
CommonCircuitPythonSocketType,
SupportsRecvInto,
SupportsRecvWithFlags,
Protocol,
): # pylint: disable=too-many-ancestors
"""Describes the structure every modern CircuitPython socket type must have."""

...

class StandardPythonSocketType(
CommonSocketType, SupportsRecvInto, SupportsRecvWithFlags, Protocol
):
"""Describes the structure every standard Python socket type must have."""

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)
def connect(self, address: Union[Tuple[Any, ...], str, bytes]) -> None:
"""Connect to a remote socket at the provided address."""
...

SocketType = Union[
LegacyCircuitPythonSocketType,
CircuitPythonSocketType,
StandardPythonSocketType,
]

SocketpoolModuleType = ModuleType

class InterfaceType(Protocol):
"""Describes the structure every interface type must have."""

@property
def TLS_MODE(self) -> int: # pylint: disable=invalid-name
"""Constant representing that a socket's connection mode is TLS."""
...
Comment on lines +140 to +146
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discovered CircuitPython does not appear to support class variables, because previously I wrote this Protocol class using this more concise form:

class InterfaceType(Protocol):
    """Describes the structure every interface type must have."""
    
    TLS_MODE: int

but that resulted in a SyntaxError: invalid syntax exception being raised for the TLS_MODE: int line.

Luckily Protocol supports an alternative syntax for these kinds of variables (specifically for Python 2.7-3.5) using property which I was able to leverage here.


SSLContextType = Union[SSLContext, "_FakeSSLContext"]

except ImportError:
pass

# CircuitPython 6.0 does not have the bytearray.split method.
# This function emulates buf.split(needle)[0], which is the functionality
Expand Down Expand Up @@ -157,7 +238,7 @@ def _recv_into(self, buf: bytearray, size: int = 0) -> int:
read_size = len(b)
buf[:read_size] = b
return read_size
return self.socket.recv_into(buf, size)
return cast("SupportsRecvInto", self.socket).recv_into(buf, size)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cast() is needed for mypy to understand that we can safely call recv_into() on self.socket here because the legacy type of CircuitPython socket (represented by the LegacyCircuitPythonSocketType and the self._backwards_compatible flag checked above being True) does not have a recv_into() method.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevincon it seems that the change here caused the library to stop working on microcontrollers with this error:

code.py output:
Connecting to AP...
Connected to WBurn-2G 	RSSI: -50
Fetching text from http://wifitest.adafruit.com/testwifi/index.html
Traceback (most recent call last):
  File "code.py", line 58, in <module>
  File "adafruit_requests.py", line 847, in get
  File "adafruit_requests.py", line 719, in request
  File "adafruit_requests.py", line 210, in __init__
  File "adafruit_requests.py", line 297, in _readto
  File "adafruit_requests.py", line 241, in _recv_into
TypeError: function takes 2 positional arguments but 3 were given

Code done running.

I think we'll need to revert it for now. Curious if you have any ideas that could make it worth in both environments?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually maybe this is not what is causing the error. My mistake. I reverted it in my local copy, but still seeing that error


@staticmethod
def _find(buf: bytes, needle: bytes, start: int, end: int) -> int:
Expand Down Expand Up @@ -440,7 +521,7 @@ def _free_sockets(self) -> None:

def _get_socket(
self, host: str, port: int, proto: str, *, timeout: float = 1
) -> SocketType:
) -> CircuitPythonSocketType:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was needed because the recv_into() method is called on the socket returned by this _get_socket() method in the request() method, and not every SocketType provides a recv_into() method (specifically LegacyCircuitPythonSocketType does not). CircuitPythonSocketType meets that requirement while also being a valid SocketType to support the socket being passed to other methods which require a SocketType socket.

# pylint: disable=too-many-branches
key = (host, port, proto)
if key in self._open_sockets:
Expand Down Expand Up @@ -693,15 +774,15 @@ def delete(self, url: str, **kw) -> Response:


class _FakeSSLSocket:
def __init__(self, socket: SocketType, tls_mode: int) -> None:
def __init__(self, socket: CircuitPythonSocketType, tls_mode: int) -> None:
self._socket = socket
self._mode = tls_mode
self.settimeout = socket.settimeout
self.send = socket.send
self.recv = socket.recv
self.close = socket.close

def connect(self, address: Union[bytes, str]) -> None:
def connect(self, address: Tuple[str, int]) -> None:
"""connect wrapper to add non-standard mode parameter"""
try:
return self._socket.connect(address, self._mode)
Expand All @@ -714,7 +795,7 @@ def __init__(self, iface: InterfaceType) -> None:
self._iface = iface

def wrap_socket(
self, socket: SocketType, server_hostname: Optional[str] = None
self, socket: CircuitPythonSocketType, server_hostname: Optional[str] = None
) -> _FakeSSLSocket:
"""Return the same socket"""
# pylint: disable=unused-argument
Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
# Uncomment the below if you use native CircuitPython modules such as
# digitalio, micropython and busio. List the modules you use. Without it, the
# autodoc module docs will fail to generate with a warning.
autodoc_mock_imports = ["adafruit_esp32spi", "adafruit_wiznet5k", "adafruit_fona"]
# autodoc_mock_imports = ["digitalio", "busio"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we no longer use those imports, I put back what this was before #87.



intersphinx_mapping = {
Expand Down
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
# Author details
author="Adafruit Industries",
author_email="circuitpython@adafruit.com",
python_requires=">=3.8",
install_requires=["Adafruit-Blinka"],
# Choose your license
license="MIT",
Expand All @@ -44,8 +45,7 @@
"Topic :: System :: Hardware",
"License :: OSI Approved :: MIT License",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.4",
"Programming Language :: Python :: 3.5",
"Programming Language :: Python :: 3.8",
],
# What does your project relate to?
keywords="adafruit blinka circuitpython micropython requests requests, networking",
Expand Down
11 changes: 11 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# SPDX-FileCopyrightText: 2022 Kevin Conley
#
# SPDX-License-Identifier: MIT

[tox]
envlist = py38

[testenv]
changedir = {toxinidir}/tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I included this changedir so that we run the tests from the tests/ folder to help ensure that adafruit_requests gets imported from the installed package in tox's virtual environment instead of being imported locally from the root of the project folder. I believe that should result in the tests importing adafruit_requests in a manner more representative of how CPython users would if they installed the package via pip.

deps = pytest==6.2.5
commands = pytest