feat(robot-server): Implement storage for "enable/disable error recov…

…ery" setting (#16333)

robot-server/robot_server/persistence/_migrations/v6_to_v7.py

@@ -3,6 +3,7 @@
 Summary of changes from schema 6:
 
 - Adds a new command_intent to store the commands intent in the commands table
+- Adds the `boolean_setting` table.
 """
 
 import json

robot-server/robot_server/persistence/file_and_directory_names.py

@@ -8,7 +8,7 @@
 
 from typing import Final
 
-LATEST_VERSION_DIRECTORY: Final = "7"
+LATEST_VERSION_DIRECTORY: Final = "7.1"
 
 DECK_CONFIGURATION_FILE: Final = "deck_configuration.json"
 PROTOCOLS_DIRECTORY: Final = "protocols"

robot-server/robot_server/persistence/persistence_directory.py

@@ -52,6 +52,9 @@ async def prepare_active_subdirectory(prepared_root: Path) -> Path:
             v3_to_v4.Migration3to4(subdirectory="4"),
             v4_to_v5.Migration4to5(subdirectory="5"),
             v5_to_v6.Migration5to6(subdirectory="6"),
+            # Subdirectory "7" was previously used on our edge branch for an in-dev
+            # schema that was never released to the public. It may be present on
+            # internal robots.
             v6_to_v7.Migration6to7(subdirectory=LATEST_VERSION_DIRECTORY),
         ],
         temp_file_prefix="temp-",

robot-server/robot_server/persistence/tables/__init__.py

@@ -12,8 +12,10 @@
     action_table,
     run_csv_rtp_table,
     data_files_table,
+    boolean_setting_table,
     PrimitiveParamSQLEnum,
     ProtocolKindSQLEnum,
+    BooleanSettingKey,
 )
 
 
@@ -28,6 +30,8 @@
     "action_table",
     "run_csv_rtp_table",
     "data_files_table",
+    "boolean_setting_table",
     "PrimitiveParamSQLEnum",
     "ProtocolKindSQLEnum",
+    "BooleanSettingKey",
 ]

robot-server/robot_server/persistence/tables/schema_7.py

@@ -102,6 +102,7 @@ class ProtocolKindSQLEnum(enum.Enum):
             PrimitiveParamSQLEnum,
             values_callable=lambda obj: [e.value for e in obj],
             create_constraint=True,
+            # todo(mm, 2024-09-24): Can we add validate_strings=True here?
         ),
         nullable=False,
     ),
@@ -263,3 +264,30 @@ class ProtocolKindSQLEnum(enum.Enum):
         nullable=True,
     ),
 )
+
+
+class BooleanSettingKey(enum.Enum):
+    """Keys for boolean settings."""
+
+    DISABLE_ERROR_RECOVERY = "disable_error_recovery"
+
+
+boolean_setting_table = sqlalchemy.Table(
+    "boolean_setting",
+    metadata,
+    sqlalchemy.Column(
+        "key",
+        sqlalchemy.Enum(
+            BooleanSettingKey,
+            values_callable=lambda obj: [e.value for e in obj],
+            validate_strings=True,
+            create_constraint=True,
+        ),
+        primary_key=True,
+    ),
+    sqlalchemy.Column(
+        "value",
+        sqlalchemy.Boolean,
+        nullable=False,
+    ),
+)

robot-server/robot_server/runs/error_recovery_setting_store.py

@@ -0,0 +1,46 @@
+# noqa: D100
+
+
+import sqlalchemy
+
+from robot_server.persistence.tables import boolean_setting_table, BooleanSettingKey
+
+
+class ErrorRecoverySettingStore:
+    """Persistently stores settings related to error recovery."""
+
+    def __init__(self, sql_engine: sqlalchemy.engine.Engine) -> None:
+        self._sql_engine = sql_engine
+
+    def get_is_disabled(self) -> bool | None:
+        """Get the value of the "error recovery enabled" setting.
+
+        `None` is the default, i.e. it's never been explicitly set one way or the other.
+        """
+        with self._sql_engine.begin() as transaction:
+            return transaction.execute(
+                sqlalchemy.select(boolean_setting_table.c.value).where(
+                    boolean_setting_table.c.key
+                    == BooleanSettingKey.DISABLE_ERROR_RECOVERY
+                )
+            ).scalar_one_or_none()
+
+    def set_is_disabled(self, is_disabled: bool | None) -> None:
+        """Set the value of the "error recovery enabled" setting.
+
+        `None` means revert to the default.
+        """
+        with self._sql_engine.begin() as transaction:
+            transaction.execute(
+                sqlalchemy.delete(boolean_setting_table).where(
+                    boolean_setting_table.c.key
+                    == BooleanSettingKey.DISABLE_ERROR_RECOVERY
+                )
+            )
+            if is_disabled is not None:
+                transaction.execute(
+                    sqlalchemy.insert(boolean_setting_table).values(
+                        key=BooleanSettingKey.DISABLE_ERROR_RECOVERY,
+                        value=is_disabled,
+                    )
+                )

robot-server/tests/integration/http_api/persistence/test_reset.py

@@ -30,23 +30,13 @@ def _get_corrupt_persistence_dir() -> Path:
 async def _assert_reset_was_successful(
     robot_client: RobotClient, persistence_directory: Path
 ) -> None:
-    # It should have no protocols.
+    # We really want to check that the server's persistence directory has been wiped
+    # clean, but testing that directly would rely on internal implementation details
+    # of file layout and tend to be brittle.
+    # As an approximation, just check that there are no protocols or runs left.
     assert (await robot_client.get_protocols()).json()["data"] == []
-
-    # It should have no runs.
     assert (await robot_client.get_runs()).json()["data"] == []
 
-    # There should be no files except for robot_server.db
-    # and an empty protocols/ directory.
-    all_files_and_directories = set(persistence_directory.glob("**/*"))
-    expected_files_and_directories = {
-        persistence_directory / "robot_server.db",
-        persistence_directory / "7",
-        persistence_directory / "7" / "protocols",
-        persistence_directory / "7" / "robot_server.db",
-    }
-    assert all_files_and_directories == expected_files_and_directories
-
 
 async def _wait_until_initialization_failed(robot_client: RobotClient) -> None:
     """Wait until the server returns an "initialization failed" health status."""

robot-server/tests/persistence/test_tables.py

@@ -142,8 +142,20 @@
         FOREIGN KEY(file_id) REFERENCES data_files (id)
     )
     """,
+    """
+    CREATE TABLE boolean_setting (
+        "key" VARCHAR(22) NOT NULL,
+        value BOOLEAN NOT NULL,
+        PRIMARY KEY ("key"),
+        CONSTRAINT booleansettingkey CHECK ("key" IN ('disable_error_recovery'))
+    )
+    """,
 ]
 
+
+EXPECTED_STATEMENTS_V7 = EXPECTED_STATEMENTS_LATEST
+
+
 EXPECTED_STATEMENTS_V6 = [
     """
     CREATE TABLE protocol (
@@ -257,8 +269,6 @@
 ]
 
 
-EXPECTED_STATEMENTS_V7 = EXPECTED_STATEMENTS_LATEST
-
 EXPECTED_STATEMENTS_V5 = [
     """
     CREATE TABLE protocol (
@@ -334,6 +344,7 @@
     """,
 ]
 
+
 EXPECTED_STATEMENTS_V4 = [
     """
     CREATE TABLE protocol (

robot-server/tests/runs/test_error_recovery_setting_store.py

@@ -0,0 +1,29 @@
+"""Tests for error_recovery_setting_store."""
+
+
+from robot_server.runs.error_recovery_setting_store import ErrorRecoverySettingStore
+
+import pytest
+import sqlalchemy
+
+
+@pytest.fixture
+def subject(
+    sql_engine: sqlalchemy.engine.Engine,
+) -> ErrorRecoverySettingStore:
+    """Return a test subject."""
+    return ErrorRecoverySettingStore(sql_engine=sql_engine)
+
+
+def test_error_recovery_setting_store(subject: ErrorRecoverySettingStore) -> None:
+    """Test `ErrorRecoverySettingStore`."""
+    assert subject.get_is_disabled() is None
+
+    subject.set_is_disabled(is_disabled=False)
+    assert subject.get_is_disabled() is False
+
+    subject.set_is_disabled(is_disabled=True)
+    assert subject.get_is_disabled() is True
+
+    subject.set_is_disabled(is_disabled=None)
+    assert subject.get_is_disabled() is None