chore(api): handle performance-metrics package not existing #14922
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes EXEC-186 If the gantry is not homed and a powercycle occurs, drop-tip wizard cannot proceed with flows. An error is raised during the flow, and ultimately a home command is dispatched that has the side effect of potentially aspirating liquid into the pipette, damaging it. We special case home errors to prevent this. The primary functional difference is now that any time an error occurs, exiting the wizard via the header should not home the gantry. Homing as a result of an error should only occur when the "Confirm removal and home" button is presented and clicked.
DerekMaggio
changed the title
DerekMaggio
changed the title
DerekMaggio
commented
Apr 17, 2024
Comment on lines
+17
to
+22
| try: | ||
| from performance_metrics import RobotContextTracker | ||
|
|
||
| return RobotContextTracker | ||
| except ImportError: | ||
| return StubbedTracker |
There was a problem hiding this comment.
Should I check opentrons.config.IS_ROBOT here?
if IS_ROBOT:
try:
from performance_metrics import RobotContextTracker
except
<raise or log here>
else:
return StubbedTracker
There was a problem hiding this comment.
nah I think this is totally fine
# Overview Closes https://opentrons.atlassian.net/browse/EXEC-399 Evaluate robot context as `In Analysis` when _analyze is running in `opentrons.cli.analyze` # Changelog - Add `performance_metrics_data` directory - Inside of performance_helpers.py - Add logic to determine if tracking should be enabled (based off FF) - Add `track_analysis` factory decorator (I think that is the right name for it lol) - Wrap `opentrons.cli.analyze._analyze` with `track_analysis` decorator - Change attribute naming on SupportsTracking to `storage_location`. If the passed Path wants to be a dir that needs to have a file defined or a direct file path, is an implementation detail. - Have `RobotContextTracker` define its own file name # Test Plan - Verifying storage with tests in test_cli.py. A follow-up PR will implement storing to a file on the robot. At that point, testing will be done on the robot. - Did verify that `performance_metrics_data` directory is getting created # Review requests None # Risk assessment Low, although this is wrapping production code, it is gated by a feature flag that defaults to false
sfoster1
approved these changes
Apr 17, 2024
Comment on lines
+17
to
+22
| try: | ||
| from performance_metrics import RobotContextTracker | ||
|
|
||
| return RobotContextTracker | ||
| except ImportError: | ||
| return StubbedTracker |
There was a problem hiding this comment.
nah I think this is totally fine
Carlos-fernandez
pushed a commit
that referenced
this pull request
May 20, 2024
# Overview Closes https://opentrons.atlassian.net/browse/EXEC-407 We do not want performance-metrics to be a required dependency for the publicly available opentrons package on PyPI. But we want to utilize performance-metrics when running the opentrons package on the robot itself. The issue is, that since the performance-metrics package uses decorators to track running code, the decorators will always need to exist. This PR handles the case where the performance-metrics package does not exist and injects stubs where necessary. # Changelog - Created the `SupportsTracking` mypy Protocol to define an interface both `RobotContextTracker` and the stubbed class, `StubbedTracker` implement - Added performance-metrics as a dev dependency so tests using performance-metrics can still run - Created `performance_helpers.py` - Contains `StubbedTracker` defininition - Handles loading `StubberTracker` if performance-metrics library fails to load - Provides `_get_robot_context_tracker` private singleton function for eventual public-facing functions to use. # Test Plan - Testing to ensure stubbed `track` decorator returns the decorated function unchanged - Validate singleton logic of _get_robot_context_tracker # Risk assessment Low, still not actually being used anywhere --------- Co-authored-by: Jethary Rader <[email protected]> Co-authored-by: Jamey Huffnagle <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Closes https://opentrons.atlassian.net/browse/EXEC-407
We do not want performance-metrics to be a required dependency for the publicly available opentrons package on PyPI. But we want to utilize performance-metrics when running the opentrons package on the robot itself.
The issue is, that since the performance-metrics package uses decorators to track running code, the decorators will always need to exist. This PR handles the case where the performance-metrics package does not exist and injects stubs where necessary.
Changelog
SupportsTrackingmypy Protocol to define an interface bothRobotContextTrackerand the stubbed class,StubbedTrackerimplementperformance_helpers.pyStubbedTrackerdefininitionStubberTrackerif performance-metrics library fails to load_get_robot_context_trackerprivate singleton function for eventual public-facing functions to use.Test Plan
trackdecorator returns the decorated function unchangedRisk assessment
Low, still not actually being used anywhere