chore(python): clean up python deps to add anyio #8228

mcous · 2021-08-10T22:06:41Z

Overview

The CPX team has run into too many walls with asyncio, so we're bringing in anyio so we can have:

Better mangement of background tasks with task cleanup
Async file reading/writing
Generally nicer async APIs

It's been a while since we've added a fresh Python dep to the robot, and our dependencies are fairly messy at this point. I took the opportunity to try to clean things up and make sure each of these was aligned for all of our Python projects:

Pipfile
Pipfile.lock
setup.py
Config.in
buildroot/package/python-***

Changes require accompanying buildroot PR: Opentrons/buildroot#131

Changelog

Upgrade pipenv so I can stop wiping out the lockfile everytime I need to change something
Add anyio to api and robot-server
Remove unused dependencies and dev dependencies from api/Pipfile
- clint (dev)
- dill (dev)
- jsonrpcserver (prod)
Upgrade typing-extensions to 3.10.0.0
Upgrade uvicorn to 0.14.0
Upgrade wsproto to 1.0.0
Lock a bunch of * depedencies in the Pipfiles
Add / correct a bunch of cross-monorepo file dependencies
Correct misalignment between Pipfile and setup.py
Correct misalignment between setup.py and Config.in
Update Dockerfile to ensure everything still works after missing deps were added to api/setup.py

Review requests

Please check a box and sign off if you complete one of these tests)

Smoke tests against Docker emulator (@mcous, resulting in 7b0fd6f)
Dockerfile changes are sensible (@amitlissack 👍'd this PR after I asked him to look at the Dockerfile)
Smoke test against hardware
- Regular robot-server smoke testing via protocols, etc (@SyntaxColoring)
- Update server smoke testing via downgrading back to production (@mcous)
Make sure development environment is good (lint + test) and the dev server runs with editable installs of local projects
- macOS (@mcous)
- Windows (@mcous)
- Linux (@mcous)

This PR (or rather, the companion PR over at Opentrons/buildroot#131) adds a new dependency that can be really helpful for some quick smoke testing of Python dependencies: importlib_metadata. With the OS build of this branch loaded on your robot: SSH in and load a python REPL:

from importlib_metadata import version

# check opentrons package
version('opentrons')
# > 4.5.0

# check uvicorn, upgraded by this PR
version('uvicorn')
# > 0.14.0

# check newly added anyio package
version('anyio')
# > 3.3.0

You can also try out anyio specifically in the REPL using anyio's hello world example

from anyio import run

async def main():
    print('Hello, world!')

run(main)

Risk assessment

High, changes to production dependencies (when combined with BR PR)

codecov · 2021-08-10T22:37:17Z

Codecov Report

Merging #8228 (30fae68) into edge (a2cd0ff) will increase coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             edge    #8228      +/-   ##
==========================================
+ Coverage   74.78%   74.91%   +0.13%     
==========================================
  Files        1661     1695      +34     
  Lines       44316    44918     +602     
  Branches     4433     4513      +80     
==========================================
+ Hits        33140    33652     +512     
- Misses      10423    10501      +78     
- Partials      753      765      +12

Flag	Coverage Δ
api	`85.86% <ø> (+0.08%)`	⬆️
app	`70.89% <ø> (+0.25%)`	⬆️
components	`48.69% <ø> (ø)`
notify-server	`89.49% <ø> (?)`
protocol-designer	`44.15% <ø> (ø)`
robot-server	`93.48% <ø> (ø)`
shared-data	`83.53% <ø> (ø)`
step-generation	`90.33% <ø> (ø)`
update-server	`74.12% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/src/opentrons/api/__init__.py	`53.76% <0.00%> (-46.24%)`	⬇️
api/src/opentrons/__init__.py
notify-server/notify_server/models/event.py	`100.00% <0.00%> (ø)`
...p/utils/getModuleTypesThatRequireExtraAttention.ts	`100.00% <0.00%> (ø)`
notify-server/notify_server/logging.py	`85.71% <0.00%> (ø)`
notify-server/notify_server/main.py	`92.30% <0.00%> (ø)`
...er/notify_server/models/hardware_event/__init__.py	`100.00% <0.00%> (ø)`
notify-server/notify_server/server/__init__.py	`100.00% <0.00%> (ø)`
...erver/notify_server/models/hardware_event/names.py	`100.00% <0.00%> (ø)`
.../RunSetupCard/ModuleSetup/MultipleModulesModal.tsx	`50.00% <0.00%> (ø)`
... and 35 more

sfoster1

Nice cleanup!

robot-server/Pipfile

update-server/Pipfile

mcous · 2021-08-12T14:20:14Z

.github/actions/python/setup/action.yaml

@@ -9,11 +9,13 @@ runs:
  steps:
    - shell: bash
      run: |
-        [[ "${OSTYPE}" =~ "linux" ]] && sudo sed -i 's/azure\.//' /etc/apt/sources.list && sudo apt-get update && sudo apt-get install -y --no-install-recommends libsystemd-dev || echo "do nothing and avoid pipefail"
+        if [[ "${OSTYPE}" =~ "linux" ]]; then


This change made because the install of libsystemd-dev was failing in our Linux CI builds, which causes the later pipenv sync to fail somewhat obscurely when trying to install systemd-python.

The failure (GH Actions logs are gone, don't know why) looked a lot like the justification for adding this sed in the first place: some Microsoft owned entry in the list was causing apt to error out. So, I made two changes:

Removed the sed ... /etc/apt/sources.list since it didn't appear to be doing anything

Moved from an ... && ... || ... chain to an explicit if block so:

This step continues to be skipped without prejudice on macOS and Windows

This step fails loudly (rather than silently) on Linux builds if something goes wrong with apt-get install

I'm pretty sure this was flakiness on the server end (there were 500s and 409s in the logs), so I don't think these changes fixed anything. However, I still thing they're sensible changes

amitlissack

Looks good!

mcous · 2021-08-12T14:23:24Z

Dockerfile

+ENV TZ=Etc/UTC
+RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone
+RUN apt-get update && apt-get install --yes python3 pip pkg-config libsystemd-dev


Other than adding apt-get install ... libsystemd-dev, I'm not sure why I needed to make any of these changes to get the Docker build working. Someone please check my work! With the changes, I can successfully build and run the server + emulator images

it looks good to me; would bet that pkg-config is a runtime configuration dep for libsystemd-dev, and the timezone stuff is Eternal Docker Containers Aren't Good At Being General Purpose Computing Environments worries

mcous · 2021-08-12T14:28:23Z

robot-server/setup.py

@@ -30,7 +30,7 @@ def get_version():

 VERSION = get_version()

-DISTNAME = "robotserver"
+DISTNAME = "robot_server"


Changing to match actual name used in import statement in the source

mcous · 2021-08-12T14:29:35Z

update-server/Pipfile

@@ -19,10 +21,6 @@ pytest-cov = "<2.11.0"
 pytest-aiohttp = "*"
 twine = ">=1.12.0"
 wheel = "==0.30.0"
-coverage = "==4.4.2"
-colorama = "*"


Did not add the atomicwrites and colorama deps to update server because this particular project is not able to run cross-platform

SyntaxColoring

Smoke-tested by running a protocol on a robot, toggling random switches, etc. Seems to work!

sanni-t

🍾

mcous added 2 commits August 10, 2021 16:07

wip: clean up python deps to add anyio

fe4f229

fixup: correct python package names

bd5a57e

mcous added chore robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). python General Python tickets and PRs; e.g. tech debt or Python guild activities labels Aug 10, 2021

mcous mentioned this pull request Aug 10, 2021

Upgrade Python dependencies of Opentrons packages Opentrons/buildroot#131

Merged

1 task

mcous added 3 commits August 10, 2021 19:30

fixup: wtf microsoft

d69e288

fixup: revert unnecessary formatting changes

5b1339e

fixup: ensure pytest windows deps are in the lockfile

273c7fb

sfoster1 approved these changes Aug 11, 2021

View reviewed changes

mcous marked this pull request as ready for review August 11, 2021 18:30

mcous requested review from a team as code owners August 11, 2021 18:30

mcous requested review from sanni-t and removed request for a team August 11, 2021 18:30

fixup: ensure docker env can still build and run

7b0fd6f

mcous commented Aug 12, 2021

View reviewed changes

robot-server/Pipfile Outdated Show resolved Hide resolved

update-server/Pipfile Outdated Show resolved Hide resolved

mcous commented Aug 12, 2021

View reviewed changes

amitlissack approved these changes Aug 12, 2021

View reviewed changes

mcous commented Aug 12, 2021

View reviewed changes

mcous added 2 commits August 12, 2021 10:30

fixup: match package names in Pipfiles with names in setup.py

36708cc

fixup: add .opentrons_config directory to eslintignore

5f69760

mattwelch added this to the CPX Sprint 39 milestone Aug 24, 2021

mcous mentioned this pull request Aug 25, 2021

feat: Upgrade version of pydantic? #8134

Closed

SyntaxColoring approved these changes Aug 26, 2021

View reviewed changes

fixup: lol lockfiles

3044f98

fixup: final typos + lockfiles pass

30fae68

sanni-t approved these changes Aug 26, 2021

View reviewed changes

mcous merged commit 781342c into edge Aug 26, 2021

mcous mentioned this pull request Aug 26, 2021

Python tests: Explicitly mark async functions instead of relying on pytest-aiohttp magic #8176

Closed

3 tasks

mcous deleted the python_dep-maintenance branch August 26, 2021 22:40

mcous mentioned this pull request Aug 27, 2021

Robot Server: Remove wsproto workaround #8266

Closed

2 tasks

sanni-t mentioned this pull request Sep 1, 2021

chore(robot-server): update dist name in makefile #8276

Merged

mcous mentioned this pull request Sep 28, 2021

fix(api): loosen numpy spec in setup.py #8418

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(python): clean up python deps to add anyio #8228

chore(python): clean up python deps to add anyio #8228

mcous commented Aug 10, 2021 •

edited

Loading

codecov bot commented Aug 10, 2021 •

edited

Loading

sfoster1 left a comment

mcous Aug 12, 2021 •

edited

Loading

amitlissack left a comment

mcous Aug 12, 2021

sfoster1 Aug 24, 2021

mcous Aug 12, 2021

mcous Aug 12, 2021

SyntaxColoring left a comment

sanni-t left a comment

chore(python): clean up python deps to add anyio #8228

chore(python): clean up python deps to add anyio #8228

Conversation

mcous commented Aug 10, 2021 • edited Loading

Overview

Changelog

Review requests

Risk assessment

codecov bot commented Aug 10, 2021 • edited Loading

Codecov Report

sfoster1 left a comment

Choose a reason for hiding this comment

mcous Aug 12, 2021 • edited Loading

Choose a reason for hiding this comment

amitlissack left a comment

Choose a reason for hiding this comment

mcous Aug 12, 2021

Choose a reason for hiding this comment

sfoster1 Aug 24, 2021

Choose a reason for hiding this comment

mcous Aug 12, 2021

Choose a reason for hiding this comment

mcous Aug 12, 2021

Choose a reason for hiding this comment

SyntaxColoring left a comment

Choose a reason for hiding this comment

sanni-t left a comment

Choose a reason for hiding this comment

mcous commented Aug 10, 2021 •

edited

Loading

codecov bot commented Aug 10, 2021 •

edited

Loading

mcous Aug 12, 2021 •

edited

Loading