Skip to content
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

fix(api,robot-server): upgrade pydantic, upgrade and add test dev deps #6673

Merged
merged 3 commits into from
Oct 7, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Oct 5, 2020

Overview

This PR is here to solve a few issues in our Python stack, one of which is blocking and the rest of which are just annoying.

Main reason for PR:

  • We need (want?) Pydantic available in api to move neutral protocol engine stuff into api where it can live closer to other protocol-related stuff

Additional stuff in PR:

Why did I lump a bunch of disparate stuff into one PR? Because every single call to pipenv install on my machine takes 2 minutes and most of them require me to rm -rf Pipfile.lock to get the commands to actually succeed. I decided to put them all in one PR for my own sanity and because lockfile changes are scary and I don't want to deal with them more than once.

Changelog

See above.

Review requests

  • All tests and checks should still pass locally
  • Pydantic upgrade most affects robot-server, so whatever smoke tests you can do are helpful there

Risk assessment

Medium, in the grand scheme of things. Deep dependency issues are problems that we've run into in the past. Does not affect production dependencies on the OT-2 because those are controlled by buildroot

@mcous mcous added api Affects the `api` project chore WIP update server shared data Affects the `shared-data` project dependencies Pull requests that update a dependency file robot server Affects the `robot-server` project labels Oct 5, 2020
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

❗ No coverage uploaded for pull request base (edge@eb906a3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #6673   +/-   ##
=======================================
  Coverage        ?   80.44%           
=======================================
  Files           ?      248           
  Lines           ?    21221           
  Branches        ?        0           
=======================================
  Hits            ?    17072           
  Misses          ?     4149           
  Partials        ?        0           
Impacted Files Coverage Δ
...pdate-server/otupdate/buildroot/name_management.py 29.50% <ø> (ø)
...rver/robot/calibration/tip_length/state_machine.py 92.30% <0.00%> (ø)
opentrons/drivers/mag_deck/driver.py 37.55% <0.00%> (ø)
.../robot/calibration/pipette_offset/state_machine.py 92.30% <0.00%> (ø)
...rver/robot_server/service/pipette_offset/models.py 100.00% <0.00%> (ø)
opentrons/protocols/api_support/util.py 91.26% <0.00%> (ø)
...r/robot_server/robot/calibration/deck/constants.py 95.45% <0.00%> (ø)
opentrons/protocols/execution/json_dispatchers.py 85.71% <0.00%> (ø)
opentrons/legacy_api/robot/mover.py 88.63% <0.00%> (ø)
...rver/robot_server/service/session/models/common.py 100.00% <0.00%> (ø)
... and 239 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb906a3...c522e36. Read the comment docs.

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

See comment, need to keep the api (and shared-data, though no changes here require it)'s setup.py in sync with the non-dev package dependencies for pypi distribution

api/Pipfile Outdated Show resolved Hide resolved
@mcous mcous added ready for review and removed WIP labels Oct 6, 2020
@mcous mcous marked this pull request as ready for review October 6, 2020 15:47
@mcous mcous requested review from a team as code owners October 6, 2020 15:47
@mcous mcous requested review from SyntaxColoring and amitlissack and removed request for a team October 6, 2020 15:47
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -15,7 +15,8 @@ dill = "==0.2.7.1"
mypy = "==0.770"
numpydoc = "==0.9.1"
pylama = "*"
pytest = "==5.4.3"
pytest = "==6.1.0"
pytest-watch = "~=4.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@mcous mcous merged commit 2e4ad6a into edge Oct 7, 2020
@mcous mcous deleted the chore_upgrade-python-deps branch October 7, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the `api` project chore dependencies Pull requests that update a dependency file robot server Affects the `robot-server` project shared data Affects the `shared-data` project update server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants