Skip to content

Commit

Permalink
chore(api): handle performance-metrics package not existing (#14922)
Browse files Browse the repository at this point in the history
# 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]>
  • Loading branch information
3 people authored Apr 17, 2024
1 parent aae8a10 commit 0940e7c
Show file tree
Hide file tree
Showing 14 changed files with 1,176 additions and 710 deletions.
1 change: 1 addition & 0 deletions api/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ pytest-profiling = "~=1.7.0"
# TODO(mc, 2022-03-31): upgrade sphinx, remove this subdep pin
jinja2 = ">=2.3,<3.1"
hypothesis = "==6.96.1"
performance-metrics = {file = "../performance-metrics", editable = true}
1,027 changes: 636 additions & 391 deletions api/Pipfile.lock

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions api/src/opentrons/cli/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
)

from opentrons_shared_data.robot.dev_types import RobotType
from opentrons.util.performance_helpers import track_analysis


@click.command()
Expand Down Expand Up @@ -63,6 +64,7 @@ def _get_input_files(files_and_dirs: Sequence[Path]) -> List[Path]:
return results


@track_analysis
async def _analyze(
files_and_dirs: Sequence[Path],
json_output: Optional[AsyncPath],
Expand Down
11 changes: 11 additions & 0 deletions api/src/opentrons/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ class ConfigElement(NamedTuple):
ConfigElementType.DIR,
"The dir where module calibration is stored",
),
ConfigElement(
"performance_metrics_dir",
"Performance Metrics Directory",
Path("performance_metrics_data"),
ConfigElementType.DIR,
"The dir where performance metrics are stored",
),
)
#: The available configuration file elements to modify. All of these can be
#: changed by editing opentrons.json, where the keys are the name elements,
Expand Down Expand Up @@ -602,3 +609,7 @@ def get_tip_length_cal_path() -> Path:

def get_custom_tiprack_def_path() -> Path:
return get_opentrons_path("custom_tiprack_dir")


def get_performance_metrics_data_dir() -> Path:
return get_opentrons_path("performance_metrics_dir")
76 changes: 76 additions & 0 deletions api/src/opentrons/util/performance_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"""Performance helpers for tracking robot context."""

from pathlib import Path
from opentrons_shared_data.performance.dev_types import (
SupportsTracking,
F,
RobotContextState,
)
from opentrons_shared_data.robot.dev_types import RobotTypeEnum
from typing import Callable, Type
from opentrons.config import (
feature_flags as ff,
get_performance_metrics_data_dir,
robot_configs,
)


_should_track = ff.enable_performance_metrics(
RobotTypeEnum.robot_literal_to_enum(robot_configs.load().model)
)


def _handle_package_import() -> Type[SupportsTracking]:
"""Handle the import of the performance_metrics package.
If the package is not available, return a stubbed tracker.
"""
try:
from performance_metrics import RobotContextTracker

return RobotContextTracker
except ImportError:
return StubbedTracker


package_to_use = _handle_package_import()
_robot_context_tracker: SupportsTracking | None = None


class StubbedTracker(SupportsTracking):
"""A stubbed tracker that does nothing."""

def __init__(self, storage_location: Path, should_track: bool) -> None:
"""Initialize the stubbed tracker."""
pass

def track(self, state: RobotContextState) -> Callable[[F], F]:
"""Return the function unchanged."""

def inner_decorator(func: F) -> F:
"""Return the function unchanged."""
return func

return inner_decorator

def store(self) -> None:
"""Do nothing."""
pass


def _get_robot_context_tracker() -> SupportsTracking:
"""Singleton for the robot context tracker."""
global _robot_context_tracker
if _robot_context_tracker is None:
# TODO: replace with path lookup and should_store lookup
_robot_context_tracker = package_to_use(
get_performance_metrics_data_dir(), _should_track
)
return _robot_context_tracker


def track_analysis(func: F) -> F:
"""Track the analysis of a protocol."""
return _get_robot_context_tracker().track(RobotContextState.ANALYZING_PROTOCOL)(
func
)
34 changes: 33 additions & 1 deletion api/tests/opentrons/cli/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
"""Test cli execution."""


import json
import tempfile
import textwrap
Expand All @@ -9,8 +11,17 @@

import pytest
from click.testing import CliRunner
from opentrons.util.performance_helpers import _get_robot_context_tracker


from opentrons.cli.analyze import analyze
# Enable tracking for the RobotContextTracker
# This must come before the import of the analyze CLI
context_tracker = _get_robot_context_tracker()

# Ignore the type error for the next line, as we're setting a private attribute for testing purposes
context_tracker._should_track = True # type: ignore[attr-defined]

from opentrons.cli.analyze import analyze # noqa: E402


def _list_fixtures(version: int) -> Iterator[Path]:
Expand Down Expand Up @@ -242,3 +253,24 @@ def test_python_error_line_numbers(
assert result.json_output is not None
[error] = result.json_output["errors"]
assert error["detail"] == expected_detail


def test_track_analysis(tmp_path: Path) -> None:
"""Test that the RobotContextTracker tracks analysis."""
protocol_source = textwrap.dedent(
"""
requirements = {"apiLevel": "2.15"}
def run(protocol):
pass
"""
)

protocol_source_file = tmp_path / "protocol.py"
protocol_source_file.write_text(protocol_source, encoding="utf-8")

before_analysis = len(context_tracker._storage) # type: ignore[attr-defined]

_get_analysis_result([protocol_source_file])

assert len(context_tracker._storage) == before_analysis + 1 # type: ignore[attr-defined]
28 changes: 28 additions & 0 deletions api/tests/opentrons/util/test_performance_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"""Tests for performance_helpers."""

from pathlib import Path
from opentrons_shared_data.performance.dev_types import RobotContextState
from opentrons.util.performance_helpers import (
StubbedTracker,
_get_robot_context_tracker,
)


def test_return_function_unchanged() -> None:
"""Test that the function is returned unchanged when using StubbedTracker."""
tracker = StubbedTracker(Path("/path/to/storage"), True)

def func_to_track() -> None:
pass

assert (
tracker.track(RobotContextState.ANALYZING_PROTOCOL)(func_to_track)
is func_to_track
)


def test_singleton_tracker() -> None:
"""Test that the tracker is a singleton."""
tracker = _get_robot_context_tracker()
tracker2 = _get_robot_context_tracker()
assert tracker is tracker2
4 changes: 4 additions & 0 deletions performance-metrics/src/performance_metrics/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
"""Opentrons performance metrics library."""

from .robot_context_tracker import RobotContextTracker

__all__ = ["RobotContextTracker"]
34 changes: 1 addition & 33 deletions performance-metrics/src/performance_metrics/datashapes.py
Original file line number Diff line number Diff line change
@@ -1,40 +1,8 @@
"""Defines data classes and enums used in the performance metrics module."""

from enum import Enum
import dataclasses
from typing import Tuple


class RobotContextState(Enum):
"""Enum representing different states of a robot's operation context."""

STARTING_UP = 0, "STARTING_UP"
CALIBRATING = 1, "CALIBRATING"
ANALYZING_PROTOCOL = 2, "ANALYZING_PROTOCOL"
RUNNING_PROTOCOL = 3, "RUNNING_PROTOCOL"
SHUTTING_DOWN = 4, "SHUTTING_DOWN"

def __init__(self, state_id: int, state_name: str) -> None:
self.state_id = state_id
self.state_name = state_name

@classmethod
def from_id(cls, state_id: int) -> "RobotContextState":
"""Returns the enum member matching the given state ID.
Args:
state_id: The ID of the state to retrieve.
Returns:
RobotContextStates: The enum member corresponding to the given ID.
Raises:
ValueError: If no matching state is found.
"""
for state in RobotContextState:
if state.state_id == state_id:
return state
raise ValueError(f"Invalid state id: {state_id}")
from opentrons_shared_data.performance.dev_types import RobotContextState


@dataclasses.dataclass(frozen=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@

from typing_extensions import ParamSpec
from collections import deque
from performance_metrics.datashapes import RawContextData, RobotContextState
from performance_metrics.datashapes import (
RawContextData,
)
from opentrons_shared_data.performance.dev_types import (
RobotContextState,
SupportsTracking,
)

P = ParamSpec("P")
R = TypeVar("R")
Expand All @@ -38,13 +44,15 @@ def _get_timing_function() -> Callable[[], int]:
timing_function = _get_timing_function()


class RobotContextTracker:
class RobotContextTracker(SupportsTracking):
"""Tracks and stores robot context and execution duration for different operations."""

def __init__(self, storage_file_path: Path, should_track: bool = False) -> None:
FILE_NAME = "context_data.csv"

def __init__(self, storage_location: Path, should_track: bool = False) -> None:
"""Initializes the RobotContextTracker with an empty storage list."""
self._storage: deque[RawContextData] = deque()
self._storage_file_path = storage_file_path
self._storage_file_path = storage_location / self.FILE_NAME
self._should_track = should_track

def track(self, state: RobotContextState) -> Callable: # type: ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from pathlib import Path
import pytest
from performance_metrics.robot_context_tracker import RobotContextTracker
from performance_metrics.datashapes import RobotContextState
from opentrons_shared_data.performance.dev_types import RobotContextState
from time import sleep, time_ns
from unittest.mock import patch

Expand All @@ -19,7 +19,7 @@
@pytest.fixture
def robot_context_tracker(tmp_path: Path) -> RobotContextTracker:
"""Fixture to provide a fresh instance of RobotContextTracker for each test."""
return RobotContextTracker(storage_file_path=tmp_path, should_track=True)
return RobotContextTracker(storage_location=tmp_path, should_track=True)


def test_robot_context_tracker(robot_context_tracker: RobotContextTracker) -> None:
Expand Down Expand Up @@ -242,8 +242,7 @@ def operation_without_tracking() -> None:

async def test_storing_to_file(tmp_path: Path) -> None:
"""Tests storing the tracked data to a file."""
file_path = tmp_path / "test_file.csv"
robot_context_tracker = RobotContextTracker(file_path, should_track=True)
robot_context_tracker = RobotContextTracker(tmp_path, should_track=True)

@robot_context_tracker.track(state=RobotContextState.STARTING_UP)
def starting_robot() -> None:
Expand All @@ -263,7 +262,7 @@ def analyzing_protocol() -> None:

robot_context_tracker.store()

with open(file_path, "r") as file:
with open(robot_context_tracker._storage_file_path, "r") as file:
lines = file.readlines()
assert (
len(lines) == 4
Expand Down
1 change: 1 addition & 0 deletions robot-server/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ sqlalchemy2-stubs = "==0.0.2a21"
# limited by tavern
python-box = "==6.1.0"
types-paho-mqtt = "==1.6.0.20240106"
performance-metrics = {file = "../performance-metrics", editable = true}

[packages]
anyio = "==3.7.1"
Expand Down
Loading

0 comments on commit 0940e7c

Please sign in to comment.