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

Exec 372 hide performance metrics project behind ff #14811

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
}
Loading