Skip to content

Commit

Permalink
Exec 372 hide performance metrics project behind ff (#14811)
Browse files Browse the repository at this point in the history
# Overview

Add feature flag for Performance Metrics project. 
Closes https://opentrons.atlassian.net/browse/EXEC-372

# Changelog 

- Add enablePerformanceMetrics feature flag in advanced settings
- Add migration function
- Update tests
  • Loading branch information
DerekMaggio authored Apr 8, 2024
1 parent 88c3f2c commit 1a5052c
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 8 deletions.
22 changes: 22 additions & 0 deletions api/src/opentrons/config/advanced_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,17 @@ class Setting(NamedTuple):
robot_type=[RobotTypeEnum.FLEX],
internal_only=True,
),
SettingDefinition(
_id="enablePerformanceMetrics",
title="Enable performance metrics",
description=(
"Do not enable."
" This is an Opentrons internal setting to collect performance metrics."
" Do not turn this on unless you are playing with the performance metrics system."
),
robot_type=[RobotTypeEnum.OT2, RobotTypeEnum.FLEX],
internal_only=True,
),
]

if (
Expand Down Expand Up @@ -709,6 +720,16 @@ def _migrate31to32(previous: SettingsMap) -> SettingsMap:
return newmap


def _migrate32to33(previous: SettingsMap) -> SettingsMap:
"""Migrate to version 33 of the feature flags file.
- Adds the enablePerformanceMetrics config element.
"""
newmap = {k: v for k, v in previous.items()}
newmap["enablePerformanceMetrics"] = None
return newmap


_MIGRATIONS = [
_migrate0to1,
_migrate1to2,
Expand Down Expand Up @@ -742,6 +763,7 @@ def _migrate31to32(previous: SettingsMap) -> SettingsMap:
_migrate29to30,
_migrate30to31,
_migrate31to32,
_migrate32to33,
]
"""
List of all migrations to apply, indexed by (version - 1). See _migrate below
Expand Down
4 changes: 4 additions & 0 deletions api/src/opentrons/config/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,7 @@ def enable_error_recovery_experiments() -> bool:
return advs.get_setting_with_env_overload(
"enableErrorRecoveryExperiments", RobotTypeEnum.FLEX
)


def enable_performance_metrics(robot_type: RobotTypeEnum) -> bool:
return advs.get_setting_with_env_overload("enablePerformanceMetrics", robot_type)
23 changes: 16 additions & 7 deletions api/tests/opentrons/config/test_advanced_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ def mock_settings_values_flex() -> Dict[str, Optional[bool]]:
}


@pytest.fixture
def mock_settings_values_flex_all() -> Dict[str, Optional[bool]]:
return {
s.id: False
for s in advanced_settings.settings
if RobotTypeEnum.FLEX in s.robot_type
}


@pytest.fixture
def mock_settings_values_empty() -> Dict[str, Optional[bool]]:
return {s.id: None for s in advanced_settings.settings}
Expand All @@ -57,25 +66,25 @@ def mock_settings(

@pytest.fixture
def mock_read_settings_file_ot2(
mock_settings_values_ot2: Dict[str, Optional[bool]],
mock_settings_values_ot2_all: Dict[str, Optional[bool]],
mock_settings_version: int,
) -> Generator[MagicMock, None, None]:
with patch("opentrons.config.advanced_settings._read_settings_file") as p:
p.return_value = advanced_settings.SettingsData(
settings_map=mock_settings_values_ot2,
settings_map=mock_settings_values_ot2_all,
version=mock_settings_version,
)
yield p


@pytest.fixture
def mock_read_settings_file_flex(
mock_settings_values_flex: Dict[str, Optional[bool]],
mock_settings_values_flex_all: Dict[str, Optional[bool]],
mock_settings_version: int,
) -> Generator[MagicMock, None, None]:
with patch("opentrons.config.advanced_settings._read_settings_file") as p:
p.return_value = advanced_settings.SettingsData(
settings_map=mock_settings_values_flex,
settings_map=mock_settings_values_flex_all,
version=mock_settings_version,
)
yield p
Expand Down Expand Up @@ -168,19 +177,19 @@ def test_get_all_adv_settings_empty(

async def test_set_adv_setting(
mock_read_settings_file_ot2: MagicMock,
mock_settings_values_ot2: MagicMock,
mock_settings_values_ot2_all: MagicMock,
mock_write_settings_file: MagicMock,
mock_settings_version: int,
restore_restart_required: None,
) -> None:
for k, v in mock_settings_values_ot2.items():
for k, v in mock_settings_values_ot2_all.items():
# Toggle the advanced setting
await advanced_settings.set_adv_setting(k, not v)
mock_write_settings_file.assert_called_with(
# Only the current key is toggled
{
nk: nv if nk != k else not v
for nk, nv in mock_settings_values_ot2.items()
for nk, nv in mock_settings_values_ot2_all.items()
},
mock_settings_version,
CONFIG["feature_flags_file"],
Expand Down
17 changes: 16 additions & 1 deletion api/tests/opentrons/config/test_advanced_settings_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

@pytest.fixture
def migrated_file_version() -> int:
return 32
return 33


# make sure to set a boolean value in default_file_settings only if
Expand All @@ -31,6 +31,7 @@ def default_file_settings() -> Dict[str, Any]:
"estopNotRequired": None,
"enableErrorRecoveryExperiments": None,
"enableOEMMode": None,
"enablePerformanceMetrics": None,
}


Expand Down Expand Up @@ -392,6 +393,18 @@ def v32_config(v31_config: Dict[str, Any]) -> Dict[str, Any]:
return r


@pytest.fixture
def v33_config(v32_config: Dict[str, Any]) -> Dict[str, Any]:
r = v32_config.copy()
r.update(
{
"_version": 33,
"enablePerformanceMetrics": None,
}
)
return r


@pytest.fixture(
scope="session",
params=[
Expand Down Expand Up @@ -429,6 +442,7 @@ def v32_config(v31_config: Dict[str, Any]) -> Dict[str, Any]:
lazy_fixture("v30_config"),
lazy_fixture("v31_config"),
lazy_fixture("v32_config"),
lazy_fixture("v33_config"),
],
)
def old_settings(request: SubRequest) -> Dict[str, Any]:
Expand Down Expand Up @@ -522,4 +536,5 @@ def test_ensures_config() -> None:
"disableOverpressureDetection": None,
"enableErrorRecoveryExperiments": None,
"enableOEMMode": None,
"enablePerformanceMetrics": None,
}

0 comments on commit 1a5052c

Please sign in to comment.