Skip to content

Commit

Permalink
chore(api): disable legacy calibration accessors on flex (#13513)
Browse files Browse the repository at this point in the history
Make the legacy routes for tip_length and pipette_offset 503 on flex and
not try and get the offsets using the old calibration accessors, since
they are hardcoded to use the ot2 versions of calibration which do not
apply to the flex and dumped errors in the logs all the time.

the api side change supports testing.
  • Loading branch information
sfoster1 authored Sep 12, 2023
1 parent 1dccc06 commit a1cde96
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 15 deletions.
3 changes: 2 additions & 1 deletion api/src/opentrons/calibration_storage/ot2/pipette_offset.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ def get_pipette_offset(
**io.read_cal_file(pipette_calibration_filepath)
)
except FileNotFoundError:
log.warning(f"Calibrations for {pipette_id} on {mount} does not exist.")
log.debug(f"Calibrations for {pipette_id} on {mount} does not exist.")
return None
except (json.JSONDecodeError, ValidationError):
log.warning(
f"Malformed calibrations for {pipette_id} on {mount}. Please factory reset your calibrations."
)
# TODO: Delete the bad calibration here maybe?
return None


Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/calibration_storage/ot2/tip_length.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def tip_lengths_for_pipette(
pass
return tip_lengths
except FileNotFoundError:
log.warning(f"Tip length calibrations not found for {pipette_id}")
log.debug(f"Tip length calibrations not found for {pipette_id}")
return tip_lengths


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def get_pipette_offset(
**io.read_cal_file(pipette_calibration_filepath)
)
except FileNotFoundError:
log.warning(f"Calibrations for {pipette_id} on {mount} does not exist.")
log.debug(f"Calibrations for {pipette_id} on {mount} does not exist.")
return None
except (json.JSONDecodeError, ValidationError):
log.warning(
Expand Down
7 changes: 5 additions & 2 deletions api/src/opentrons/calibration_storage/ot3/tip_length.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,22 @@ def tip_lengths_for_pipette(
) -> typing.Dict[str, v1.TipLengthModel]:
tip_lengths = {}
try:
# While you technically could drop some data in for tip length calibration on the flex,
# it is not necessary and there is no UI frontend for it, so this code will mostly be
# taking the FileNotFoundError path.
tip_length_filepath = config.get_tip_length_cal_path() / f"{pipette_id}.json"
all_tip_lengths_for_pipette = io.read_cal_file(tip_length_filepath)
for tiprack, data in all_tip_lengths_for_pipette.items():
try:
tip_lengths[tiprack] = v1.TipLengthModel(**data)
except (json.JSONDecodeError, ValidationError):
log.warning(
log.debug(
f"Tip length calibration is malformed for {tiprack} on {pipette_id}"
)
pass
return tip_lengths
except FileNotFoundError:
log.warning(f"Tip length calibrations not found for {pipette_id}")
# this is the overwhelmingly common case
return tip_lengths


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
from opentrons.types import Point, Mount

# TODO change this when imports are fixed
from opentrons.calibration_storage.ot3.pipette_offset import save_pipette_calibration
from opentrons.calibration_storage.ot3.pipette_offset import (
save_pipette_calibration,
get_pipette_offset,
)
from opentrons.calibration_storage import (
types as cal_top_types,
get_pipette_offset,
ot3_gripper_offset,
)
from opentrons.hardware_control.types import OT3Mount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,5 @@ def _get_calibrated_tip_length_sync(
f"No calibrated tip length found for {pipette_serial},"
f" using nominal fallback value of {nominal_fallback}"
)
log.warning(message, exc_info=e)
log.debug(message, exc_info=e)
return nominal_fallback
14 changes: 13 additions & 1 deletion robot-server/robot_server/hardware.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
feature_flags as ff,
)
from opentrons.util.helpers import utc_now
from opentrons.hardware_control import ThreadManagedHardware, HardwareControlAPI
from opentrons.hardware_control import ThreadManagedHardware, HardwareControlAPI, API
from opentrons.hardware_control.simulator_setup import load_simulator_thread_manager
from opentrons.hardware_control.types import (
HardwareEvent,
Expand All @@ -50,6 +50,7 @@
)
from .errors.robot_errors import (
NotSupportedOnOT2,
NotSupportedOnFlex,
HardwareNotYetInitialized,
HardwareFailedToInitialize,
)
Expand Down Expand Up @@ -226,6 +227,17 @@ def get_ot3_hardware(
return cast(OT3API, thread_manager.wrapped())


def get_ot2_hardware(
thread_manager: ThreadManagedHardware = Depends(get_thread_manager),
) -> "API":
"""Get an OT2 hardware controller."""
if not thread_manager.wraps_instance(API):
raise NotSupportedOnFlex(
detail="This route is only available on an OT-2."
).as_error(status.HTTP_403_FORBIDDEN)
return cast(API, thread_manager.wrapped())


async def get_firmware_update_manager(
app_state: AppState = Depends(get_app_state),
thread_manager: ThreadManagedHardware = Depends(get_thread_manager),
Expand Down
11 changes: 8 additions & 3 deletions robot-server/robot_server/service/pipette_offset/router.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
from starlette import status
from fastapi import APIRouter
from fastapi import APIRouter, Depends
from typing import Optional

from opentrons import types as ot_types
from opentrons.calibration_storage.ot2 import pipette_offset, models

from robot_server.hardware import get_ot2_hardware
from robot_server.errors import ErrorBody
from robot_server.service.pipette_offset import models as pip_models
from robot_server.service.errors import RobotServerError, CommonErrorDef
from robot_server.service.shared_models import calibration as cal_model

from opentrons.hardware_control import API

router = APIRouter()


Expand Down Expand Up @@ -45,7 +48,9 @@ def _format_calibration(
response_model=pip_models.MultipleCalibrationsResponse,
)
async def get_all_pipette_offset_calibrations(
pipette_id: Optional[str] = None, mount: Optional[pip_models.MountType] = None
pipette_id: Optional[str] = None,
mount: Optional[pip_models.MountType] = None,
_: API = Depends(get_ot2_hardware),
) -> pip_models.MultipleCalibrationsResponse:

all_calibrations = pipette_offset.get_all_pipette_offset_calibrations()
Expand Down Expand Up @@ -75,7 +80,7 @@ async def get_all_pipette_offset_calibrations(
responses={status.HTTP_404_NOT_FOUND: {"model": ErrorBody}},
)
async def delete_specific_pipette_offset_calibration(
pipette_id: str, mount: pip_models.MountType
pipette_id: str, mount: pip_models.MountType, _: API = Depends(get_ot2_hardware)
):
try:
pipette_offset.delete_pipette_offset_file(
Expand Down
10 changes: 8 additions & 2 deletions robot-server/robot_server/service/tip_length/router.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
from starlette import status
from fastapi import APIRouter
from fastapi import APIRouter, Depends
from typing import Optional

from opentrons.calibration_storage import types as cal_types
from opentrons.calibration_storage.ot2 import tip_length, models

from robot_server.hardware import get_ot2_hardware
from robot_server.errors import ErrorBody
from robot_server.service.tip_length import models as tl_models
from robot_server.service.errors import RobotServerError, CommonErrorDef
from robot_server.service.shared_models import calibration as cal_model

from opentrons.hardware_control import API


router = APIRouter()

Expand Down Expand Up @@ -48,6 +51,7 @@ async def get_all_tip_length_calibrations(
tiprack_hash: Optional[str] = None,
pipette_id: Optional[str] = None,
tiprack_uri: Optional[str] = None,
_: API = Depends(get_ot2_hardware),
) -> tl_models.MultipleCalibrationsResponse:
all_calibrations = tip_length.get_all_tip_length_calibrations()
if not all_calibrations:
Expand Down Expand Up @@ -79,7 +83,9 @@ async def get_all_tip_length_calibrations(
"serial and tiprack hash",
responses={status.HTTP_404_NOT_FOUND: {"model": ErrorBody}},
)
async def delete_specific_tip_length_calibration(tiprack_hash: str, pipette_id: str):
async def delete_specific_tip_length_calibration(
tiprack_hash: str, pipette_id: str, _: API = Depends(get_ot2_hardware)
):
try:
tip_length.delete_tip_length_calibration(tiprack_hash, pipette_id)
except cal_types.TipLengthCalNotFound:
Expand Down
14 changes: 13 additions & 1 deletion robot-server/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from opentrons.types import Point, Mount

from robot_server import app
from robot_server.hardware import get_hardware
from robot_server.hardware import get_hardware, get_ot2_hardware
from robot_server.versioning import API_VERSION_HEADER, LATEST_API_VERSION_HEADER_VALUE
from robot_server.service.session.manager import SessionManager
from robot_server.persistence import get_sql_engine, create_sql_engine
Expand Down Expand Up @@ -139,11 +139,23 @@ async def get_version_override() -> ComponentVersions:
del app.dependency_overrides[get_versions]


@pytest.fixture
def _override_ot2_hardware_with_mock(hardware: MagicMock) -> Iterator[None]:
async def get_ot2_hardware_override() -> API:
"""Override for the get_ot2_hardware FastAPI dependency."""
return MagicMock(spec=API)

app.dependency_overrides[get_ot2_hardware] = get_ot2_hardware_override
yield
del app.dependency_overrides[get_ot2_hardware]


@pytest.fixture
def api_client(
_override_hardware_with_mock: None,
_override_sql_engine_with_mock: None,
_override_version_with_mock: None,
_override_ot2_hardware_with_mock: None,
) -> TestClient:
client = TestClient(app)
client.headers.update({API_VERSION_HEADER: LATEST_API_VERSION_HEADER_VALUE})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,27 @@ stages:
mount: 'right'
response:
status_code: 200

---
test_name: Pipette calibrations inaccessible on flex
marks:
- ot3_only
- usefixtures:
- ot3_server_base_url
- set_up_pipette_offset_temp_directory
stages:
- name: GET request 403s
request:
url: "{ot3_server_base_url}/calibration/pipette_offset"
method: GET
response:
status_code: 403
- name: DELETE request 403s
request:
url: "{ot3_server_base_url}/calibration/pipette_offset"
method: DELETE
params:
pipette_id: '321'
mount: 'right'
response:
status_code: 403
19 changes: 19 additions & 0 deletions robot-server/tests/integration/test_tip_length_access.tavern.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,22 @@ stages:
method: DELETE
response:
status_code: 404
---
test_name: Tip length inaccessible on flex
marks:
- ot3_only
- usefixtures:
- ot3_server_base_url
stages:
- name: GET request 403s
request:
url: "{ot3_server_base_url}/calibration/tip_length"
method: GET
response:
status_code: 403
- name: DELETE request 403s
request:
url: "{ot3_server_base_url}/calibration/tip_length"
method: DELETE
response:
status_code: 403

0 comments on commit a1cde96

Please sign in to comment.