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(app,api): Add an "awaiting-recovery" run status #14651

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented Mar 13, 2024

Overview

Closes EXEC-299.

Changelog

  • Set up our types for a new awaiting-recovery run status. It's not actually used by Protocol Engine or emitted by robot-server yet. See the Python docstring for the semantics that we have in mind so far.
  • Attempt to update the many little bits of logic in the app that use the run status.

Test plan

There's nothing substantial to test here yet because the new status isn't used by Protocol Engine or emitted by robot-server yet.

I've deliberately omitted unit tests because I think, at this stage, we want to keep it cheap to fundamentally rework the semantics of this status. But I'm happy to go back and add them if anyone thinks they'd be helpful.

Review requests

Any questions or suggestions about the semantics of this new status?

Risk assessment

Low until this new status actually gets emitted.

@SyntaxColoring SyntaxColoring requested review from a team as code owners March 13, 2024 17:25
@SyntaxColoring SyntaxColoring requested review from jerader and removed request for a team March 13, 2024 17:25
@SyntaxColoring SyntaxColoring requested review from mjhuff and a team March 13, 2024 17:26
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.34%. Comparing base (f307641) to head (3d7ec4f).
Report is 3 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #14651      +/-   ##
==========================================
- Coverage   67.34%   67.34%   -0.01%     
==========================================
  Files        2485     2485              
  Lines       71439    71438       -1     
  Branches     9057     9056       -1     
==========================================
- Hits        48114    48113       -1     
  Misses      21173    21173              
  Partials     2152     2152              
Flag Coverage Δ
g-code-testing 92.43% <ø> (ø)

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

Files Coverage Δ
...pi/src/opentrons/protocol_engine/state/commands.py 99.44% <ø> (ø)
api/src/opentrons/protocol_engine/types.py 97.50% <ø> (ø)
...rganisms/Devices/ProtocolRun/ProtocolRunHeader.tsx 88.12% <ø> (ø)
...rc/organisms/Devices/hooks/useLastRunCommandKey.ts 22.22% <ø> (ø)
app/src/organisms/Devices/hooks/useRunStatuses.ts 100.00% <ø> (ø)

Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

Looks like you got all the relevant spots in the app! This LGTM.

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.

Python code looks good to me and so does the js changes although I think someone a little more in tune with how the app works probably should take a look there. We should pretty rapidly follow this up with re-adding the ability to cancel from this state at least.

@SyntaxColoring SyntaxColoring merged commit 1e18315 into edge Mar 14, 2024
38 checks passed
@SyntaxColoring SyntaxColoring deleted the error_recovery_run_status branch March 14, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants