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

feat(api): add analysis context tracking #14928

Conversation

DerekMaggio
Copy link
Contributor

@DerekMaggio DerekMaggio commented Apr 16, 2024

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

@DerekMaggio DerekMaggio changed the base branch from edge to EXEC-407-handle-performance-metrics-package-not-existing April 16, 2024 21:24
@DerekMaggio DerekMaggio changed the title Exec 399 track analysis feat: add analysis context tracking Apr 16, 2024
@DerekMaggio DerekMaggio changed the title feat: add analysis context tracking feat(api): add analysis context tracking Apr 17, 2024
@DerekMaggio DerekMaggio marked this pull request as ready for review April 17, 2024 14:13
@DerekMaggio DerekMaggio requested a review from a team as a code owner April 17, 2024 14:13
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!

@DerekMaggio DerekMaggio merged commit 65176b7 into EXEC-407-handle-performance-metrics-package-not-existing Apr 17, 2024
59 checks passed
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.

2 participants