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

refactor(api): add create_file_runner factory #7857

Merged
merged 5 commits into from
Jun 4, 2021

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Jun 1, 2021

Overview

This PR is 3 of 4 working towards #7816.

  1. chore(api,robot-server): update testing dev deps chore(api,robot-server): update testing dev deps #7855 Merged
  2. refactor(robot-server): add experimental protocol router refactor(robot-server): add experimental protocol router #7856 Merged
  3. refactor(api): add create_file_runner factory refactor(api): add create_file_runner factory #7857
  4. refactor(robot-server): start wiring /sessions into runner and engine refactor(robot-server): start wiring /sessions into runner and engine #7858

This PR prepares for session wireup to the JsonFileRunner by adding a factory function to set up a JsonFileRunner.

Blocked by #7856, will require sync after merge

Changelog

  • Addded create_file_runner factory to create a file runner instance given:
    • file_type: Python or JSON (Python and others left unimplemented)
    • file_path: path on the filesystem to the file
    • engine: ProtocolEngine instance

Review requests

Risk assessment

Low, not hooked to production code

@mcous mcous added the robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). label Jun 1, 2021
@mcous mcous requested review from a team and X-sam and removed request for a team June 1, 2021 14:34
@mcous mcous requested a review from a team as a code owner June 1, 2021 14:34
@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #7857 (6071275) into edge (708fa36) will decrease coverage by 9.80%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #7857      +/-   ##
==========================================
- Coverage   93.44%   83.63%   -9.81%     
==========================================
  Files         130      353     +223     
  Lines        5081    21727   +16646     
==========================================
+ Hits         4748    18172   +13424     
- Misses        333     3555    +3222     
Impacted Files Coverage Δ
opentrons/system/resin.py 0.00% <0.00%> (ø)
opentrons/protocol_engine/protocol_engine.py 97.50% <0.00%> (ø)
opentrons/protocols/geometry/deck.py 92.44% <0.00%> (ø)
opentrons/protocols/context/labware.py 71.87% <0.00%> (ø)
opentrons/hardware_control/modules/update.py 28.12% <0.00%> (ø)
.../protocols/context/simulator/instrument_context.py 68.14% <0.00%> (ø)
opentrons/commands/protocol_commands.py 94.73% <0.00%> (ø)
opentrons/hardware_control/emulation/tempdeck.py 97.50% <0.00%> (ø)
opentrons/protocol_engine/state/labware.py 100.00% <0.00%> (ø)
opentrons/protocol_engine/types.py 100.00% <0.00%> (ø)
... and 213 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 708fa36...6071275. Read the comment docs.

api/src/opentrons/file_runner/create_file_runner.py Outdated Show resolved Hide resolved
api/src/opentrons/file_runner/create_file_runner.py Outdated Show resolved Hide resolved
api/src/opentrons/file_runner/create_file_runner.py Outdated Show resolved Hide resolved
api/src/opentrons/file_runner/json_file_runner.py Outdated Show resolved Hide resolved
api/src/opentrons/file_runner/protocol_file.py Outdated Show resolved Hide resolved
api/src/opentrons/file_runner/python_file_runner.py Outdated Show resolved Hide resolved
@@ -51,6 +52,12 @@ def exception_handler(loop, context):
loop.set_exception_handler(None)


@pytest.fixture
def decoy() -> Decoy:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally putting this in the top level conftest



@pytest.fixture
def json_protocol_dict(minimal_labware_def: dict) -> dict:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moving this helpful fixture that @sanni-t added

@mcous mcous force-pushed the robot-server_protocol-upload-endpoints_2 branch from b64bb80 to 086ae72 Compare June 1, 2021 17:03
@mcous mcous requested a review from a team as a code owner June 1, 2021 17:03
@mcous mcous force-pushed the api_add-create-file-runner-factory_3 branch from 48f1db4 to 069cc69 Compare June 1, 2021 17:06
@mcous mcous changed the base branch from robot-server_protocol-upload-endpoints_2 to edge June 2, 2021 00:36
@mcous mcous changed the base branch from edge to robot-server_protocol-upload-endpoints_2 June 2, 2021 00:36
Base automatically changed from robot-server_protocol-upload-endpoints_2 to edge June 2, 2021 20:08
@mcous mcous force-pushed the api_add-create-file-runner-factory_3 branch from 069cc69 to c9a98a6 Compare June 2, 2021 20:10
@sanni-t sanni-t self-requested a review June 3, 2021 22:09
@mcous
Copy link
Contributor Author

mcous commented Jun 4, 2021

Changes addressed during PR review

@mcous mcous merged commit c129c6f into edge Jun 4, 2021
@mcous mcous deleted the api_add-create-file-runner-factory_3 branch June 4, 2021 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants