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

chore(api): disable legacy calibration accessors on flex #13513

Merged
merged 4 commits into from
Sep 12, 2023
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
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}"
Comment on lines -46 to 50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might leave this as a warning, unless this is actually more false-positive than true-positive in practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just never should ever print ever so i can make it a warning

)
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),
Comment on lines 50 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure I understand correctly: on a Flex, even retrieving pipette calibrations through these endpoints was doing the wrong thing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This code was hardcoded to select the ot2 pipette offset loaders.

) -> 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
Loading