From 8dc44b0986e22b3eae19a05c130a153005d30682 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:52:01 -0500 Subject: [PATCH 01/24] fix(app): fix styling for instrument and module (#14444) Fix empty card styling for instrument cards and border radius for instrument and module cards closes RQA-2262 --- app/src/molecules/InstrumentCard/index.tsx | 2 +- .../organisms/Devices/PipetteCard/index.tsx | 23 ++++++++++--------- app/src/organisms/ModuleCard/index.tsx | 3 ++- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/app/src/molecules/InstrumentCard/index.tsx b/app/src/molecules/InstrumentCard/index.tsx index c8a32724405..727a2db041f 100644 --- a/app/src/molecules/InstrumentCard/index.tsx +++ b/app/src/molecules/InstrumentCard/index.tsx @@ -64,7 +64,7 @@ export function InstrumentCard(props: InstrumentCardProps): JSX.Element { { return ( @@ -252,22 +252,23 @@ export const PipetteCard = (props: PipetteCardProps): JSX.Element => { <> - - {pipetteModelSpecs !== null ? ( + {pipetteModelSpecs !== null ? ( + - ) : null} - - + + ) : null} + {isFlexPipetteAttached && !isPipetteCalibrated ? ( {isEstopNotDisengaged ? ( diff --git a/app/src/organisms/ModuleCard/index.tsx b/app/src/organisms/ModuleCard/index.tsx index 888a95c3510..7066fe4f18d 100644 --- a/app/src/organisms/ModuleCard/index.tsx +++ b/app/src/organisms/ModuleCard/index.tsx @@ -6,6 +6,7 @@ import { useHistory } from 'react-router-dom' import { ALIGN_START, + BORDERS, Box, COLORS, DIRECTION_COLUMN, @@ -248,7 +249,7 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => { return ( From 929186fc5ca73552777f3c2f6e7387b517874e12 Mon Sep 17 00:00:00 2001 From: Sarah Breen Date: Thu, 8 Feb 2024 16:29:12 -0500 Subject: [PATCH 02/24] fix(app): add white translucent background to module info to make text pop (#14456) fix RQA-2034 --- app/src/organisms/Devices/ModuleInfo.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/src/organisms/Devices/ModuleInfo.tsx b/app/src/organisms/Devices/ModuleInfo.tsx index 49aefd3ab3f..8bfe68ef0cb 100644 --- a/app/src/organisms/Devices/ModuleInfo.tsx +++ b/app/src/organisms/Devices/ModuleInfo.tsx @@ -17,7 +17,6 @@ import { getModuleDisplayName, getModuleDef2, MAGNETIC_BLOCK_V1, - THERMOCYCLER_MODULE_TYPE, } from '@opentrons/shared-data' import { StyledText } from '../../atoms/text' @@ -61,10 +60,7 @@ export const ModuleInfo = (props: ModuleInfoProps): JSX.Element => { width={labwareInterfaceXDimension ?? xDimension} flexProps={{ padding: SPACING.spacing16, - backgroundColor: - moduleDef.moduleType === THERMOCYCLER_MODULE_TYPE - ? COLORS.white - : COLORS.transparent, + backgroundColor: `${COLORS.white}${COLORS.opacity90HexCode}`, }} > Date: Thu, 8 Feb 2024 23:53:56 -0500 Subject: [PATCH 03/24] feat(app): Add Robot Initialization State (#14448) Closes RAUT-944 --- .../localization/en/device_settings.json | 3 +- .../RobotUpdateProgressModal.tsx | 108 ++++++++++++------ .../RobotUpdateProgressModal.test.tsx | 46 +++++++- .../RobotSettings/UpdateBuildroot/index.tsx | 23 +++- .../RobotDashboard/RecentRunProtocolCard.tsx | 24 ++-- .../__tests__/RecentRunProtocolCard.test.tsx | 21 ++++ .../resources/health/__tests__/hooks.test.ts | 45 ++++++++ app/src/resources/health/hooks.ts | 44 +++++++ react-api-client/src/health/useHealth.ts | 24 ++-- 9 files changed, 278 insertions(+), 60 deletions(-) create mode 100644 app/src/resources/health/__tests__/hooks.test.ts create mode 100644 app/src/resources/health/hooks.ts diff --git a/app/src/assets/localization/en/device_settings.json b/app/src/assets/localization/en/device_settings.json index 7223c34972e..4c62a44044a 100644 --- a/app/src/assets/localization/en/device_settings.json +++ b/app/src/assets/localization/en/device_settings.json @@ -99,7 +99,7 @@ "display_led_lights": "Status LEDs", "display_led_lights_description": "Control the strip of color lights on the front of the robot.", "display_sleep_settings": "Display Sleep Settings", - "do_not_turn_off": "This could take up to 15 minutes. Don't turn off the robot.", + "do_not_turn_off": "This could take up to {{minutes}} minutes. Don't turn off the robot.", "done": "Done", "download": "Download", "download_calibration_data": "Download calibration logs", @@ -237,6 +237,7 @@ "returns_your_device_to_new_state": "This returns your device to a new state.", "robot_busy_protocol": "This robot cannot be updated while a protocol is running on it", "robot_calibration_data": "Robot Calibration Data", + "robot_initializing": "Initializing robot...", "robot_name": "Robot Name", "robot_operating_update_available": "Robot Operating System Update Available", "robot_serial_number": "Robot Serial Number", diff --git a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/RobotUpdateProgressModal.tsx b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/RobotUpdateProgressModal.tsx index 42c32964967..bef8dce44b7 100644 --- a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/RobotUpdateProgressModal.tsx +++ b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/RobotUpdateProgressModal.tsx @@ -27,11 +27,16 @@ import { } from '../../../../redux/robot-update' import { useRobotUpdateInfo } from './useRobotUpdateInfo' import successIcon from '../../../../assets/images/icon_success.png' +import { + useRobotInitializationStatus, + INIT_STATUS, +} from '../../../../resources/health/hooks' import type { State } from '../../../../redux/types' import type { SetStatusBarCreateCommand } from '@opentrons/shared-data/protocol' import type { RobotUpdateSession } from '../../../../redux/robot-update/types' import type { UpdateStep } from './useRobotUpdateInfo' +import type { RobotInitializationStatus } from '../../../../resources/health/hooks' const UPDATE_PROGRESS_BAR_STYLE = css` margin-top: ${SPACING.spacing24}; @@ -92,34 +97,20 @@ export function RobotUpdateProgressModal({ installFromFileRef.current.click() }, [showFileSelect]) - const hasStoppedUpdating = error || updateStep === 'finished' + const robotInitStatus = useRobotInitializationStatus() + const hasRobotCompletedInit = + updateStep === 'finished' && robotInitStatus !== INIT_STATUS.INITIALIZING const letUserExitUpdate = useAllowExitIfUpdateStalled( updateStep, - progressPercent + progressPercent, + robotInitStatus + ) + const { modalBodyText, subProgressBarText } = useGetModalText( + updateStep, + letUserExitUpdate, + robotName, + robotInitStatus ) - - let modalBodyText = '' - let subProgressBarText = t('do_not_turn_off') - switch (updateStep) { - case 'initial': - case 'error': - modalBodyText = '' - break - case 'download': - modalBodyText = t('downloading_update') - break - case 'install': - modalBodyText = t('installing_update') - break - case 'restart': - modalBodyText = t('restarting_robot') - if (letUserExitUpdate) { - subProgressBarText = t('restart_taking_too_long', { robotName }) - } - break - default: - modalBodyText = t('installing_update') - } return ( ) : null } > - {hasStoppedUpdating ? ( + {hasRobotCompletedInit || error ? ( @@ -156,7 +147,9 @@ export function RobotUpdateProgressModal({ outerStyles={UPDATE_PROGRESS_BAR_STYLE} /> - {letUserExitUpdate && updateStep !== 'restart' ? ( + {letUserExitUpdate && + updateStep !== 'restart' && + updateStep !== 'finished' ? ( <> {t('problem_during_update')} {t('try_restarting_the_update')} {showFileSelect && ( @@ -232,11 +225,13 @@ function SuccessOrError({ errorMessage }: SuccessOrErrorProps): JSX.Element { ) } -export const TIME_BEFORE_ALLOWING_EXIT_MS = 600000 // 10 mins +export const TIME_BEFORE_ALLOWING_EXIT = 600000 // 10 mins +export const TIME_BEFORE_ALLOWING_EXIT_INIT = 2400000 // 40 mins. Account for tasks like DB migration. function useAllowExitIfUpdateStalled( updateStep: UpdateStep | null, - progressPercent: number + progressPercent: number, + robotInitStatus: RobotInitializationStatus ): boolean { const [letUserExitUpdate, setLetUserExitUpdate] = React.useState( false @@ -247,19 +242,21 @@ function useAllowExitIfUpdateStalled( React.useEffect(() => { if (updateStep === 'initial' && prevSeenUpdateProgress.current !== null) { prevSeenUpdateProgress.current = null - } else if (updateStep === 'finished' && exitTimeoutRef.current) { - clearTimeout(exitTimeoutRef.current) - setLetUserExitUpdate(false) } else if (progressPercent !== prevSeenUpdateProgress.current) { if (exitTimeoutRef.current) clearTimeout(exitTimeoutRef.current) exitTimeoutRef.current = setTimeout(() => { setLetUserExitUpdate(true) - }, TIME_BEFORE_ALLOWING_EXIT_MS) + }, TIME_BEFORE_ALLOWING_EXIT) prevSeenUpdateProgress.current = progressPercent setLetUserExitUpdate(false) + } else if (robotInitStatus === INIT_STATUS.INITIALIZING) { + if (exitTimeoutRef.current) clearTimeout(exitTimeoutRef.current) + exitTimeoutRef.current = setTimeout(() => { + setLetUserExitUpdate(true) + }, TIME_BEFORE_ALLOWING_EXIT_INIT) } - }, [progressPercent, updateStep]) + }, [progressPercent, updateStep, robotInitStatus]) React.useEffect(() => { return () => { @@ -313,3 +310,42 @@ function useCleanupRobotUpdateSessionOnDismount(): void { } }, []) } + +function useGetModalText( + updateStep: UpdateStep | null, + letUserExitUpdate: boolean, + robotName: string, + robotInitStatus: RobotInitializationStatus +): { modalBodyText: string; subProgressBarText: string } { + const { t } = useTranslation('device_settings') + + let modalBodyText = '' + let subProgressBarText = t('do_not_turn_off', { minutes: 15 }) + switch (updateStep) { + case 'initial': + case 'error': + modalBodyText = '' + break + case 'download': + modalBodyText = t('downloading_update') + break + case 'install': + modalBodyText = t('installing_update') + break + case 'restart': + if (robotInitStatus === INIT_STATUS.INITIALIZING) { + modalBodyText = t('robot_initializing') + subProgressBarText = t('do_not_turn_off', { minutes: 40 }) + } else { + modalBodyText = t('restarting_robot') + } + if (letUserExitUpdate) { + subProgressBarText = t('restart_taking_too_long', { robotName }) + } + break + default: + modalBodyText = t('installing_update') + } + + return { modalBodyText, subProgressBarText } +} diff --git a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/__tests__/RobotUpdateProgressModal.test.tsx b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/__tests__/RobotUpdateProgressModal.test.tsx index f91d5dea975..e8142c6c21a 100644 --- a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/__tests__/RobotUpdateProgressModal.test.tsx +++ b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/__tests__/RobotUpdateProgressModal.test.tsx @@ -5,7 +5,8 @@ import { renderWithProviders } from '@opentrons/components' import { useCreateLiveCommandMutation } from '@opentrons/react-api-client' import { RobotUpdateProgressModal, - TIME_BEFORE_ALLOWING_EXIT_MS, + TIME_BEFORE_ALLOWING_EXIT, + TIME_BEFORE_ALLOWING_EXIT_INIT, } from '../RobotUpdateProgressModal' import { useRobotUpdateInfo } from '../useRobotUpdateInfo' import { @@ -13,6 +14,10 @@ import { getRobotUpdateDownloadError, } from '../../../../../redux/robot-update' import { useDispatchStartRobotUpdate } from '../../../../../redux/robot-update/hooks' +import { + useRobotInitializationStatus, + INIT_STATUS, +} from '../../../../../resources/health/hooks' import type { SetStatusBarCreateCommand } from '@opentrons/shared-data' import type { RobotUpdateSession } from '../../../../../redux/robot-update/types' @@ -21,6 +26,7 @@ jest.mock('@opentrons/react-api-client') jest.mock('../useRobotUpdateInfo') jest.mock('../../../../../redux/robot-update') jest.mock('../../../../../redux/robot-update/hooks') +jest.mock('../../../../../resources/health/hooks') const mockUseCreateLiveCommandMutation = useCreateLiveCommandMutation as jest.MockedFunction< typeof useCreateLiveCommandMutation @@ -37,6 +43,9 @@ const mockUseDispatchStartRobotUpdate = useDispatchStartRobotUpdate as jest.Mock const mockGetRobotUpdateDownloadError = getRobotUpdateDownloadError as jest.MockedFunction< typeof getRobotUpdateDownloadError > +const mockUseRobotInitializationStatus = useRobotInitializationStatus as jest.MockedFunction< + typeof useRobotInitializationStatus +> const render = ( props: React.ComponentProps @@ -78,6 +87,7 @@ describe('DownloadUpdateModal', () => { mockGetRobotSessionIsManualFile.mockReturnValue(false) mockUseDispatchStartRobotUpdate.mockReturnValue(jest.fn) mockGetRobotUpdateDownloadError.mockReturnValue(null) + mockUseRobotInitializationStatus.mockReturnValue(INIT_STATUS.SUCCEEDED) }) afterEach(() => { @@ -188,10 +198,42 @@ describe('DownloadUpdateModal', () => { render(props) act(() => { - jest.advanceTimersByTime(TIME_BEFORE_ALLOWING_EXIT_MS) + jest.advanceTimersByTime(TIME_BEFORE_ALLOWING_EXIT) }) screen.getByText(/Try restarting the update./i) screen.getByText(/This update is taking longer than usual/i) }) + + it('renders alternative text if the robot is initializing', () => { + mockUseRobotInitializationStatus.mockReturnValue(INIT_STATUS.INITIALIZING) + mockUseRobotUpdateInfo.mockReturnValue({ + updateStep: 'restart', + progressPercent: 100, + }) + render(props) + + screen.getByText(/Initializing robot.../i) + screen.getByText( + "This could take up to 40 minutes. Don't turn off the robot." + ) + }) + + it('renders alternative text if update takes too long while robot is initializing', () => { + jest.useFakeTimers() + mockUseRobotInitializationStatus.mockReturnValue(INIT_STATUS.INITIALIZING) + mockUseRobotUpdateInfo.mockReturnValue({ + updateStep: 'restart', + progressPercent: 100, + }) + render(props) + + act(() => { + jest.advanceTimersByTime(TIME_BEFORE_ALLOWING_EXIT_INIT) + }) + + screen.getByText( + /Check the Advanced tab of its settings page to see whether it updated successfully./i + ) + }) }) diff --git a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/index.tsx b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/index.tsx index 358de1368a1..1cbbaacc0c8 100644 --- a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/index.tsx +++ b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/index.tsx @@ -2,6 +2,8 @@ import * as React from 'react' import { useSelector, useDispatch } from 'react-redux' import NiceModal, { useModal } from '@ebay/nice-modal-react' +import { ApiHostProvider } from '@opentrons/react-api-client' + import { setRobotUpdateSeen, robotUpdateIgnored, @@ -9,7 +11,8 @@ import { } from '../../../../redux/robot-update' import { ViewUpdateModal } from './ViewUpdateModal' import { RobotUpdateProgressModal } from './RobotUpdateProgressModal' -import { UNREACHABLE } from '../../../../redux/discovery' +import { UNREACHABLE, OPENTRONS_USB } from '../../../../redux/discovery' +import { appShellRequestor } from '../../../../redux/shell/remote' import type { Dispatch } from '../../../../redux/types' import type { DiscoveredRobot } from '../../../../redux/discovery/types' @@ -50,11 +53,19 @@ const UpdateBuildroot = NiceModal.create( if (hasSeenSessionOnce.current) return ( - + + + ) else if (robot != null && robot.status !== UNREACHABLE) return ( diff --git a/app/src/organisms/OnDeviceDisplay/RobotDashboard/RecentRunProtocolCard.tsx b/app/src/organisms/OnDeviceDisplay/RobotDashboard/RecentRunProtocolCard.tsx index 2404429b861..24df56978bd 100644 --- a/app/src/organisms/OnDeviceDisplay/RobotDashboard/RecentRunProtocolCard.tsx +++ b/app/src/organisms/OnDeviceDisplay/RobotDashboard/RecentRunProtocolCard.tsx @@ -16,6 +16,14 @@ import { TYPOGRAPHY, } from '@opentrons/components' import { useProtocolQuery } from '@opentrons/react-api-client' +import { + RUN_STATUS_FAILED, + RUN_STATUS_STOPPED, + RUN_STATUS_SUCCEEDED, + Run, + RunData, + RunStatus, +} from '@opentrons/api-client' import { StyledText } from '../../../atoms/text' import { Chip } from '../../../atoms/Chip' @@ -26,13 +34,10 @@ import { useMissingProtocolHardware } from '../../../pages/Protocols/hooks' import { useCloneRun } from '../../ProtocolUpload/hooks' import { useHardwareStatusText } from './hooks' import { - RUN_STATUS_FAILED, - RUN_STATUS_STOPPED, - RUN_STATUS_SUCCEEDED, - Run, - RunData, - RunStatus, -} from '@opentrons/api-client' + useRobotInitializationStatus, + INIT_STATUS, +} from '../../../resources/health/hooks' + import type { ProtocolResource } from '@opentrons/shared-data' interface RecentRunProtocolCardProps { @@ -83,6 +88,9 @@ export function ProtocolWithLastRun({ const onResetSuccess = (createRunResponse: Run): void => history.push(`runs/${createRunResponse.data.id}/setup`) const { cloneRun } = useCloneRun(runData.id, onResetSuccess) + const robotInitStatus = useRobotInitializationStatus() + const isRobotInitializing = + robotInitStatus === INIT_STATUS.INITIALIZING || robotInitStatus == null const [showSpinner, setShowSpinner] = React.useState(false) const protocolName = @@ -140,7 +148,7 @@ export function ProtocolWithLastRun({ } ).replace('about ', '') - return isProtocolFetching || isLookingForHardware ? ( + return isProtocolFetching || isLookingForHardware || isRobotInitializing ? ( const mockSkeleton = Skeleton as jest.MockedFunction +const mockUseRobotInitializationStatus = useRobotInitializationStatus as jest.MockedFunction< + typeof useRobotInitializationStatus +> const render = (props: React.ComponentProps) => { return renderWithProviders( @@ -143,6 +151,7 @@ describe('RecentRunProtocolCard', () => { when(mockUseCloneRun) .calledWith(RUN_ID, expect.anything()) .mockReturnValue({ cloneRun: mockCloneRun, isLoading: false }) + mockUseRobotInitializationStatus.mockReturnValue(INIT_STATUS.SUCCEEDED) }) afterEach(() => { @@ -231,4 +240,16 @@ describe('RecentRunProtocolCard', () => { const [{ getByText }] = render(props) getByText('mock Skeleton') }) + + it('should render the skeleton when the robot server is initializing', () => { + mockUseRobotInitializationStatus.mockReturnValue(INIT_STATUS.INITIALIZING) + const [{ getByText }] = render(props) + getByText('mock Skeleton') + }) + + it('should render the skeleton when the robot server is unresponsive', () => { + mockUseRobotInitializationStatus.mockReturnValue(null) + const [{ getByText }] = render(props) + getByText('mock Skeleton') + }) }) diff --git a/app/src/resources/health/__tests__/hooks.test.ts b/app/src/resources/health/__tests__/hooks.test.ts new file mode 100644 index 00000000000..07e60b60afb --- /dev/null +++ b/app/src/resources/health/__tests__/hooks.test.ts @@ -0,0 +1,45 @@ +import { renderHook } from '@testing-library/react' + +import { useHealthQuery } from '@opentrons/react-api-client' + +import { useRobotInitializationStatus, INIT_STATUS } from '../hooks' + +jest.mock('@opentrons/react-api-client') + +const mockUseHealthQuery = (useHealthQuery as jest.MockedFunction< + typeof useHealthQuery +>) as jest.Mock + +describe('useRobotInitializationStatus', () => { + it('should return "INITIALIZING" when response status code is 503', () => { + mockUseHealthQuery.mockImplementation(({ onSuccess }) => { + onSuccess({ status: 503 }) + }) + const { result } = renderHook(() => useRobotInitializationStatus()) + expect(result.current).toBe(INIT_STATUS.INITIALIZING) + }) + + it('should return "SUCCEEDED" when response status code is 200', () => { + mockUseHealthQuery.mockImplementation(({ onSuccess }) => { + onSuccess({ status: 200 }) + }) + const { result } = renderHook(() => useRobotInitializationStatus()) + expect(result.current).toBe(INIT_STATUS.SUCCEEDED) + }) + + it('should return "FAILED" when response status code is 500', () => { + mockUseHealthQuery.mockImplementation(({ onSuccess }) => { + onSuccess({ status: 500 }) + }) + const { result } = renderHook(() => useRobotInitializationStatus()) + expect(result.current).toBe(INIT_STATUS.FAILED) + }) + + it('should return null when response status code is not 200, 500, or 503.', () => { + mockUseHealthQuery.mockImplementation(({ onSuccess }) => { + onSuccess({ status: 404 }) + }) + const { result } = renderHook(() => useRobotInitializationStatus()) + expect(result.current).toBeNull() + }) +}) diff --git a/app/src/resources/health/hooks.ts b/app/src/resources/health/hooks.ts new file mode 100644 index 00000000000..52f522d2436 --- /dev/null +++ b/app/src/resources/health/hooks.ts @@ -0,0 +1,44 @@ +import * as React from 'react' + +import { useHealthQuery } from '@opentrons/react-api-client' + +const ROBOT_HEALTH_POLL_MS = 5000 + +export const INIT_STATUS = { + INITIALIZING: 'INITIALIZING', + SUCCEEDED: 'SUCCEEDED', + FAILED: 'FAILED', +} as const + +export type RobotInitializationStatus = + | typeof INIT_STATUS[keyof typeof INIT_STATUS] + | null + +export function useRobotInitializationStatus(): RobotInitializationStatus { + const responseStatusCode = React.useRef(null) + + useHealthQuery({ + refetchInterval: ROBOT_HEALTH_POLL_MS, + onSuccess: data => (responseStatusCode.current = data?.status ?? null), + onError: error => + (responseStatusCode.current = error.response?.status ?? null), + }) + + let status: RobotInitializationStatus + switch (responseStatusCode.current) { + case 503: + status = INIT_STATUS.INITIALIZING + break + case 200: + status = INIT_STATUS.SUCCEEDED + break + case 500: + status = INIT_STATUS.FAILED + break + default: + status = null + break + } + + return status +} diff --git a/react-api-client/src/health/useHealth.ts b/react-api-client/src/health/useHealth.ts index 067267ae500..c675f7f6cfb 100644 --- a/react-api-client/src/health/useHealth.ts +++ b/react-api-client/src/health/useHealth.ts @@ -1,18 +1,28 @@ -import { HostConfig, Health, getHealth } from '@opentrons/api-client' +import { HostConfig, getHealth } from '@opentrons/api-client' import { UseQueryResult, useQuery } from 'react-query' import { useHost } from '../api' -export function useHealthQuery(): UseQueryResult { +import type { UseQueryOptions } from 'react-query' +import type { AxiosResponse, AxiosError } from 'axios' +import type { Health } from '@opentrons/api-client' + +export function useHealthQuery( + options: UseQueryOptions, AxiosError> = {} +): UseQueryResult, AxiosError> { const host = useHost() - const query = useQuery( - ['health', host], - () => getHealth(host as HostConfig).then(response => response.data), - { enabled: host !== null } + const queryKey = ['health', host] + const query = useQuery, AxiosError>( + queryKey, + () => getHealth(host as HostConfig), + { + ...options, + enabled: host !== null && options.enabled !== false, + } ) return query } export function useHealth(): Health | undefined { - return useHealthQuery().data + return useHealthQuery().data?.data } From fd0f7aa754cfd15b2a81e91971951198929b0e92 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 9 Feb 2024 10:54:06 -0500 Subject: [PATCH 04/24] chore: use api tokens for pypi deploys (#14464) * chore: use api tokens for pypi deploys * enforce latest pip * perhaps latest pipenv --- .github/actions/python/pypi-deploy/action.yaml | 2 +- .github/actions/python/setup/action.yaml | 3 ++- .github/workflows/api-test-lint-deploy.yaml | 4 ++-- .github/workflows/shared-data-test-lint-deploy.yaml | 4 ++-- Makefile | 3 ++- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/actions/python/pypi-deploy/action.yaml b/.github/actions/python/pypi-deploy/action.yaml index 8f1d9407fe5..e24ab6e7b20 100644 --- a/.github/actions/python/pypi-deploy/action.yaml +++ b/.github/actions/python/pypi-deploy/action.yaml @@ -28,7 +28,7 @@ runs: fi fi status=0 - CI=1 QUIET=1 BUILD_NUMBER=${OT_BUILD} make -C ${{ inputs.project }} clean deploy twine_repository_url=${{ inputs.repository_url }} pypi_username=opentrons pypi_password=${{ inputs.password }} || status=$? + CI=1 QUIET=1 BUILD_NUMBER=${OT_BUILD} make -C ${{ inputs.project }} clean deploy twine_repository_url=${{ inputs.repository_url }} pypi_username=__token__ pypi_password=${{ inputs.password }} || status=$? if [[ ${status} != 0 ]] && [[ ${{ inputs.repository_url }} =~ "test.pypi.org" ]]; then echo "upload failures allowed to test pypi" exit 0 diff --git a/.github/actions/python/setup/action.yaml b/.github/actions/python/setup/action.yaml index 7fc81cc258f..8e41955e6d0 100644 --- a/.github/actions/python/setup/action.yaml +++ b/.github/actions/python/setup/action.yaml @@ -27,6 +27,7 @@ runs: - shell: bash run: | npm install --global shx@0.3.3 - $OT_PYTHON -m pip install pipenv==2023.11.15 + $OT_PYTHON -m pip install --upgrade pip + $OT_PYTHON -m pip install pipenv==2023.12.1 - shell: bash run: 'make -C ${{ inputs.project }} setup' diff --git a/.github/workflows/api-test-lint-deploy.yaml b/.github/workflows/api-test-lint-deploy.yaml index 9b4e8068ec8..5143c6e8021 100644 --- a/.github/workflows/api-test-lint-deploy.yaml +++ b/.github/workflows/api-test-lint-deploy.yaml @@ -165,11 +165,11 @@ jobs: with: project: 'api' repository_url: 'https://test.pypi.org/legacy/' - password: '${{ secrets.OT_TEST_PYPI_PASSWORD }}' + password: '${{ secrets.TEST_PYPI_DEPLOY_TOKEN_OPENTRONS }}' - if: startsWith(env.OT_TAG, 'v') name: 'upload to real pypi' uses: './.github/actions/python/pypi-deploy' with: project: 'api' repository_url: 'https://upload.pypi.org/legacy/' - password: '${{ secrets.OT_PYPI_PASSWORD }}' + password: '${{ secrets.PYPI_DEPLOY_TOKEN_OPENTRONS }}' diff --git a/.github/workflows/shared-data-test-lint-deploy.yaml b/.github/workflows/shared-data-test-lint-deploy.yaml index 97228ea2d70..3a299da66b0 100644 --- a/.github/workflows/shared-data-test-lint-deploy.yaml +++ b/.github/workflows/shared-data-test-lint-deploy.yaml @@ -179,14 +179,14 @@ jobs: with: project: 'shared-data/python' repository_url: 'https://test.pypi.org/legacy/' - password: '${{ secrets.OT_TEST_PYPI_PASSWORD }}' + password: '${{ secrets.TEST_PYPI_DEPLOY_TOKEN_OPENTRONS_SHARED_DATA }}' - if: startsWith(env.OT_TAG, 'v') name: 'upload to pypi' uses: './.github/actions/python/pypi-deploy' with: project: 'shared-data/python' repository_url: 'https://upload.pypi.org/legacy/' - password: '${{ secrets.OT_PYPI_PASSWORD }}' + password: '${{ secrets.PYPI_DEPLOY_TOKEN_OPENTRONS_SHARED_DATA }}' publish-switch: runs-on: 'ubuntu-latest' diff --git a/Makefile b/Makefile index 03c88d4ad68..a8a8c591d3e 100755 --- a/Makefile +++ b/Makefile @@ -65,7 +65,8 @@ PYTHON_SETUP_TARGETS := $(addsuffix -py-setup, $(PYTHON_DIRS)) .PHONY: setup-py setup-py: - $(OT_PYTHON) -m pip install pipenv==2023.11.15 + $(OT_PYTHON) -m pip install --upgrade pip + $(OT_PYTHON) -m pip install pipenv==2023.12.1 $(MAKE) $(PYTHON_SETUP_TARGETS) From 277074a3706c12cda0e135fb571795de2e572f68 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 9 Feb 2024 10:54:21 -0500 Subject: [PATCH 05/24] chore(docs): Add trash bin load for api 2.16 example (#14455) (#14463) Trashbin support is required for API 2.16 Co-authored-by: Anurag Kanase <79215426+anuwrag@users.noreply.github.com> --- api/docs/v2/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/api/docs/v2/index.rst b/api/docs/v2/index.rst index 92b770e4065..743bf425c91 100644 --- a/api/docs/v2/index.rst +++ b/api/docs/v2/index.rst @@ -75,6 +75,7 @@ For example, if we wanted to transfer liquid from well A1 to well B1 on a plate, # protocol run function def run(protocol: protocol_api.ProtocolContext): # labware + trash = protocol.load_trash_bin("A3") plate = protocol.load_labware( "corning_96_wellplate_360ul_flat", location="D1" ) From 8dc6f790e9d9e7db3edb9d44c024f85981e2f6e4 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 9 Feb 2024 10:56:45 -0500 Subject: [PATCH 06/24] fix(api): keep proto contents in bytes for longer (#14446) When we parse python protocols, we were doing it by (1) making it into a string with decode('utf-8') and then (2) passing the string ast.parse(). The problem with this is that decode('utf-8') does not apply "universal newlines", which means that the code object created by compiling the ast will have line numbers that are around twice what they should be under certain circumstances (windows machine, crlf file, mercury in the seventh house, etc). Then, when we go and display a nice error message about a syntax error or whatever, the user says "why is this error message pointing to a place past the end of my protocol". This should fix that by keeping the protocol contents in bytes form all the way through to passing ast.parse() a bytes that _has never been through str.decode('utf-8')_ which should preserve everything. --- api/src/opentrons/protocols/parse.py | 38 ++++++++++++++------- api/src/opentrons/protocols/types.py | 10 ++++-- api/src/opentrons/util/entrypoint_util.py | 5 ++- api/tests/opentrons/protocols/test_parse.py | 15 ++++---- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/api/src/opentrons/protocols/parse.py b/api/src/opentrons/protocols/parse.py index ee868912ed7..712b4fe4416 100644 --- a/api/src/opentrons/protocols/parse.py +++ b/api/src/opentrons/protocols/parse.py @@ -192,7 +192,9 @@ def version_from_string(vstr: str) -> APIVersion: return APIVersion(major=int(matches.group(1)), minor=int(matches.group(2))) -def _parse_json(protocol_contents: str, filename: Optional[str] = None) -> JsonProtocol: +def _parse_json( + protocol_contents: Union[str, bytes], filename: Optional[str] = None +) -> JsonProtocol: """Parse a protocol known or at least suspected to be json""" protocol_json = json.loads(protocol_contents) version, validated = validate_json(protocol_json) @@ -208,7 +210,7 @@ def _parse_json(protocol_contents: str, filename: Optional[str] = None) -> JsonP def _parse_python( - protocol_contents: str, + protocol_contents: Union[str, bytes], python_parse_mode: PythonParseMode, filename: Optional[str] = None, bundled_labware: Optional[Dict[str, "LabwareDefinition"]] = None, @@ -338,28 +340,37 @@ def parse( ) return result else: - if isinstance(protocol_file, bytes): - protocol_str = protocol_file.decode("utf-8") - else: - protocol_str = protocol_file - if filename and filename.endswith(".json"): - return _parse_json(protocol_str, filename) + return _parse_json(protocol_file, filename) elif filename and filename.endswith(".py"): return _parse_python( - protocol_contents=protocol_str, + protocol_contents=protocol_file, python_parse_mode=python_parse_mode, filename=filename, extra_labware=extra_labware, bundled_data=extra_data, ) - # our jsonschema says the top level json kind is object - if protocol_str and protocol_str[0] in ("{", b"{"): - return _parse_json(protocol_str, filename) + # our jsonschema says the top level json kind is object so we can + # rely on it starting with a { if it's valid. that could either be + # a string or bytes. + # + # if it's a string, then if the protocol file starts with a { and + # we do protocol_file[0] then we get the string "{". + # + # if it's a bytes, then if the protocol file starts with the ascii or + # utf-8 representation of { and we do protocol_file[0] we get 123, + # because while single elements of strings are strings, single elements + # of bytes are the byte value as a number. + # + # to get that number we could either use ord() or do what we do here + # which I think is a little nicer, if any of the above can be called + # "nice". + if protocol_file and protocol_file[0] in ("{", b"{"[0]): + return _parse_json(protocol_file, filename) else: return _parse_python( - protocol_contents=protocol_str, + protocol_contents=protocol_file, python_parse_mode=python_parse_mode, filename=filename, extra_labware=extra_labware, @@ -499,6 +510,7 @@ def _version_from_static_python_info( """ from_requirements = (static_python_info.requirements or {}).get("apiLevel", None) from_metadata = (static_python_info.metadata or {}).get("apiLevel", None) + requested_level = from_requirements or from_metadata if requested_level is None: return None diff --git a/api/src/opentrons/protocols/types.py b/api/src/opentrons/protocols/types.py index 792951efbfa..273a3e877d4 100644 --- a/api/src/opentrons/protocols/types.py +++ b/api/src/opentrons/protocols/types.py @@ -31,7 +31,13 @@ class StaticPythonInfo: @dataclass(frozen=True) class _ProtocolCommon: - text: str + text: Union[str, bytes] + """The original text of the protocol file in the format it was specified with. + + This leads to a wide type but it is actually quite important that we do not ever + str.decode('utf-8') this because it will break the interpreter's understanding of + line numbers for if we have to format an exception. + """ filename: Optional[str] """The original name of the main protocol file, if it had a name. @@ -74,7 +80,7 @@ class PythonProtocol(_ProtocolCommon): class BundleContents(NamedTuple): - protocol: str + protocol: Union[str, bytes] bundled_labware: Dict[str, "LabwareDefinition"] bundled_data: Dict[str, bytes] bundled_python: Dict[str, str] diff --git a/api/src/opentrons/util/entrypoint_util.py b/api/src/opentrons/util/entrypoint_util.py index 90236f568f7..63779eda18f 100644 --- a/api/src/opentrons/util/entrypoint_util.py +++ b/api/src/opentrons/util/entrypoint_util.py @@ -166,7 +166,10 @@ def adapt_protocol_source(protocol: Protocol) -> Generator[ProtocolSource, None, # through the filesystem. https://opentrons.atlassian.net/browse/RSS-281 main_file = pathlib.Path(temporary_directory) / main_file_name - main_file.write_text(protocol.text, encoding="utf-8") + if isinstance(protocol.text, str): + main_file.write_text(protocol.text, encoding="utf-8") + else: + main_file.write_bytes(protocol.text) labware_files: List[pathlib.Path] = [] if isinstance(protocol, PythonProtocol) and protocol.extra_labware is not None: diff --git a/api/tests/opentrons/protocols/test_parse.py b/api/tests/opentrons/protocols/test_parse.py index cc86621601a..11a39507238 100644 --- a/api/tests/opentrons/protocols/test_parse.py +++ b/api/tests/opentrons/protocols/test_parse.py @@ -1,6 +1,6 @@ import json from textwrap import dedent -from typing import Any, Callable, Optional, Union +from typing import Any, Callable, Optional, Union, Literal import pytest from opentrons_shared_data.robot.dev_types import RobotType @@ -407,7 +407,7 @@ def run(ctx): pass @pytest.mark.parametrize("filename", ["protocol.py", None]) def test_parse_python_details( protocol_source: str, - protocol_text_kind: str, + protocol_text_kind: Literal["str", "bytes"], filename: Optional[str], expected_api_level: APIVersion, expected_robot_type: RobotType, @@ -423,8 +423,11 @@ def test_parse_python_details( parsed = parse(text, filename) assert isinstance(parsed, PythonProtocol) - assert parsed.text == protocol_source - assert isinstance(parsed.text, str) + assert parsed.text == text + if protocol_text_kind == "str": + assert isinstance(parsed.text, str) + else: + assert isinstance(parsed.text, bytes) assert parsed.filename == filename assert parsed.contents.co_filename == ( @@ -454,13 +457,13 @@ def test_parse_json_details( get_json_protocol_fixture: Callable[..., Any], fixture_version: str, fixture_name: str, - protocol_text_kind: str, + protocol_text_kind: Literal["str", "bytes"], filename: str, ) -> None: protocol = get_json_protocol_fixture( fixture_version=fixture_version, fixture_name=fixture_name, decode=False ) - if protocol_text_kind == "text": + if protocol_text_kind == "str": protocol_text: Union[bytes, str] = protocol else: protocol_text = protocol.encode("utf-8") From a3428ad8c2f6dba873ed2dbd833f3d1fcf1ab1f8 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:05:58 -0500 Subject: [PATCH 07/24] fix(app): update useIsRobot busy hook to check for firmware update (#14457) * fix(app): update useIsRobot busy hook to check for firmware update closes RQA-2293 --- .../hooks/__tests__/useIsRobotBusy.test.ts | 37 ++++++++++++++++++- .../organisms/Devices/hooks/useIsRobotBusy.ts | 14 ++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/src/organisms/Devices/hooks/__tests__/useIsRobotBusy.test.ts b/app/src/organisms/Devices/hooks/__tests__/useIsRobotBusy.test.ts index 3057f76d168..b4f2fc4011b 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useIsRobotBusy.test.ts +++ b/app/src/organisms/Devices/hooks/__tests__/useIsRobotBusy.test.ts @@ -1,6 +1,10 @@ import { UseQueryResult } from 'react-query' -import { useAllSessionsQuery, useEstopQuery } from '@opentrons/react-api-client' +import { + useAllSessionsQuery, + useEstopQuery, + useCurrentAllSubsystemUpdatesQuery, +} from '@opentrons/react-api-client' import { DISENGAGED, @@ -43,6 +47,9 @@ const mockUseNotifyCurrentMaintenanceRun = useNotifyCurrentMaintenanceRun as jes const mockUseEstopQuery = useEstopQuery as jest.MockedFunction< typeof useEstopQuery > +const mockUseCurrentAllSubsystemUpdatesQuery = useCurrentAllSubsystemUpdatesQuery as jest.MockedFunction< + typeof useCurrentAllSubsystemUpdatesQuery +> const mockUseIsFlex = useIsFlex as jest.MockedFunction describe('useIsRobotBusy', () => { @@ -61,6 +68,18 @@ describe('useIsRobotBusy', () => { data: {}, } as any) mockUseEstopQuery.mockReturnValue({ data: mockEstopStatus } as any) + mockUseCurrentAllSubsystemUpdatesQuery.mockReturnValue({ + data: { + data: [ + { + id: '123', + createdAt: 'today', + subsystem: 'pipette_right', + updateStatus: 'done', + }, + ], + }, + } as any) mockUseIsFlex.mockReturnValue(false) }) @@ -200,4 +219,20 @@ describe('useIsRobotBusy', () => { const result = useIsRobotBusy() expect(result).toBe(true) }) + it('returns true when a subsystem update is in progress', () => { + mockUseCurrentAllSubsystemUpdatesQuery.mockReturnValue({ + data: { + data: [ + { + id: '123', + createdAt: 'today', + subsystem: 'pipette_right', + updateStatus: 'updating', + }, + ], + }, + } as any) + const result = useIsRobotBusy() + expect(result).toBe(true) + }) }) diff --git a/app/src/organisms/Devices/hooks/useIsRobotBusy.ts b/app/src/organisms/Devices/hooks/useIsRobotBusy.ts index d867f78f123..772039b22d2 100644 --- a/app/src/organisms/Devices/hooks/useIsRobotBusy.ts +++ b/app/src/organisms/Devices/hooks/useIsRobotBusy.ts @@ -2,6 +2,7 @@ import { useAllSessionsQuery, useEstopQuery, useHost, + useCurrentAllSubsystemUpdatesQuery, } from '@opentrons/react-api-client' import { useNotifyCurrentMaintenanceRun } from '../../../resources/maintenance_runs/useNotifyCurrentMaintenanceRun' @@ -33,12 +34,23 @@ export function useIsRobotBusy( ...queryOptions, enabled: isFlex, }) + const { + data: currentSubsystemsUpdatesData, + } = useCurrentAllSubsystemUpdatesQuery({ + refetchInterval: ROBOT_STATUS_POLL_MS, + }) + const isSubsystemUpdating = + currentSubsystemsUpdatesData?.data.some( + update => + update.updateStatus === 'queued' || update.updateStatus === 'updating' + ) ?? false return ( robotHasCurrentRun || isMaintenanceRunExisting || (allSessionsQueryResponse?.data?.data != null && allSessionsQueryResponse?.data?.data?.length !== 0) || - (isFlex && estopStatus?.data.status !== DISENGAGED && estopError == null) + (isFlex && estopStatus?.data.status !== DISENGAGED && estopError == null) || + isSubsystemUpdating ) } From ce940e8a544e909fea1f64da1214873a1f1f0643 Mon Sep 17 00:00:00 2001 From: Sarah Breen Date: Fri, 9 Feb 2024 11:42:24 -0500 Subject: [PATCH 08/24] fix(app): add prop so run setup pipette flows are strung together (#14460) fix RQA-2305 --- .../InstrumentMountItem/ProtocolInstrumentMountItem.tsx | 3 +++ app/src/organisms/PipetteWizardFlows/MountingPlate.tsx | 3 +++ app/src/organisms/ProtocolSetupInstruments/index.tsx | 1 + 3 files changed, 7 insertions(+) diff --git a/app/src/organisms/InstrumentMountItem/ProtocolInstrumentMountItem.tsx b/app/src/organisms/InstrumentMountItem/ProtocolInstrumentMountItem.tsx index fa356cf753a..6c614404866 100644 --- a/app/src/organisms/InstrumentMountItem/ProtocolInstrumentMountItem.tsx +++ b/app/src/organisms/InstrumentMountItem/ProtocolInstrumentMountItem.tsx @@ -19,6 +19,7 @@ import { NINETY_SIX_CHANNEL, PipetteName, SINGLE_MOUNT_PIPETTES, + LoadedPipette, } from '@opentrons/shared-data' import { SmallButton } from '../../atoms/buttons' @@ -49,6 +50,7 @@ interface ProtocolInstrumentMountItemProps { attachedInstrument: InstrumentData | null speccedName: PipetteName | GripperModel instrumentsRefetch?: () => void + pipetteInfo?: LoadedPipette[] } export function ProtocolInstrumentMountItem( props: ProtocolInstrumentMountItemProps @@ -172,6 +174,7 @@ export function ProtocolInstrumentMountItem( closeFlow={() => setShowPipetteWizardFlow(false)} selectedPipette={selectedPipette} mount={mount as Mount} + pipetteInfo={props.pipetteInfo} onComplete={props.instrumentsRefetch} /> ) : null} diff --git a/app/src/organisms/PipetteWizardFlows/MountingPlate.tsx b/app/src/organisms/PipetteWizardFlows/MountingPlate.tsx index a4bf6ed8d2f..51347bd18f4 100644 --- a/app/src/organisms/PipetteWizardFlows/MountingPlate.tsx +++ b/app/src/organisms/PipetteWizardFlows/MountingPlate.tsx @@ -5,6 +5,7 @@ import { COLORS, SPACING } from '@opentrons/components' import { StyledText } from '../../atoms/text' import { SimpleWizardBody } from '../../molecules/SimpleWizardBody' import { GenericWizardTile } from '../../molecules/GenericWizardTile' +import { InProgressModal } from '../../molecules/InProgressModal/InProgressModal' import { getPipetteAnimations96 } from './utils' import { BODY_STYLE, FLOWS, SECTIONS } from './constants' import type { PipetteWizardStepProps } from './types' @@ -13,6 +14,7 @@ export const MountingPlate = ( props: PipetteWizardStepProps ): JSX.Element | null => { const { + isRobotMoving, goBack, proceed, flowType, @@ -46,6 +48,7 @@ export const MountingPlate = ( }) } + if (isRobotMoving) return return errorMessage ? ( ) })} From d22e93db55072865f92a6b56b6a33075ac5ebca7 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Fri, 9 Feb 2024 14:42:52 -0500 Subject: [PATCH 09/24] fix(app): Fix improper transitioning run status in protocol runs (#14459) Closes RQA-2291, RQA-2307, RQA-2306, RQA-2304 * fix(app): fix non polling notify hooks not always refetching data appropriately Instead of checking the refetchInterval property to see if a notification refetch should occur, we should check if staleTime is infinity. This accurately captures the refetchHTTP behavior that we actually want. * fix(app): fix infinite cancelling run state when run status is idle->stop-requested --- app/src/organisms/RunTimeControl/hooks.ts | 8 +++-- app/src/resources/runs/useNotifyRunQuery.ts | 2 +- app/src/resources/useNotifyService.ts | 30 +++++++++++-------- react-api-client/src/runs/useAllRunsQuery.ts | 4 +++ .../robot_server/runs/run_data_manager.py | 11 ++++--- robot-server/robot_server/runs/run_store.py | 1 + .../publishers/runs_publisher.py | 21 ++++++++----- 7 files changed, 46 insertions(+), 31 deletions(-) diff --git a/app/src/organisms/RunTimeControl/hooks.ts b/app/src/organisms/RunTimeControl/hooks.ts index 7c63c55212c..1c676077d98 100644 --- a/app/src/organisms/RunTimeControl/hooks.ts +++ b/app/src/organisms/RunTimeControl/hooks.ts @@ -77,9 +77,11 @@ export function useRunStatus( refetchInterval: DEFAULT_STATUS_REFETCH_INTERVAL, enabled: lastRunStatus.current == null || - !([RUN_STATUS_FAILED, RUN_STATUS_SUCCEEDED] as RunStatus[]).includes( - lastRunStatus.current - ), + !([ + RUN_STATUS_FAILED, + RUN_STATUS_SUCCEEDED, + RUN_STATUS_STOP_REQUESTED, + ] as RunStatus[]).includes(lastRunStatus.current), onSuccess: data => (lastRunStatus.current = data?.data?.status ?? null), ...options, }) diff --git a/app/src/resources/runs/useNotifyRunQuery.ts b/app/src/resources/runs/useNotifyRunQuery.ts index 901c3c70b9e..d70298c2377 100644 --- a/app/src/resources/runs/useNotifyRunQuery.ts +++ b/app/src/resources/runs/useNotifyRunQuery.ts @@ -19,7 +19,7 @@ export function useNotifyRunQuery( const { isNotifyError } = useNotifyService({ topic: `robot-server/runs/${runId}` as NotifyTopic, refetchUsingHTTP: () => setRefetchUsingHTTP(true), - options, + options: { ...options, enabled: options.enabled && runId != null }, }) const isNotifyEnabled = !isNotifyError && !options?.forceHttpPolling diff --git a/app/src/resources/useNotifyService.ts b/app/src/resources/useNotifyService.ts index 895ebb58ac0..87398f4bc50 100644 --- a/app/src/resources/useNotifyService.ts +++ b/app/src/resources/useNotifyService.ts @@ -43,24 +43,21 @@ export function useNotifyService({ const host = useHost() const isNotifyError = React.useRef(false) const doTrackEvent = useTrackEvent() - const { enabled, refetchInterval, forceHttpPolling } = options - const isRefetchEnabled = - refetchInterval !== undefined && refetchInterval !== false + const { enabled, staleTime, forceHttpPolling } = options + const hostname = host?.hostname ?? null React.useEffect(() => { // Always fetch on initial mount. refetchUsingHTTP() - if (!forceHttpPolling && isRefetchEnabled && enabled !== false) { - const hostname = host?.hostname ?? null + if ( + !forceHttpPolling && + enabled !== false && + hostname != null && + staleTime !== Infinity + ) { const eventEmitter = appShellListener(hostname, topic) - eventEmitter.on('data', onDataListener) - - if (hostname != null) { - dispatch(notifySubscribeAction(hostname, topic)) - } else { - console.error('NotifyService expected hostname, received null.') - } + dispatch(notifySubscribeAction(hostname, topic)) return () => { eventEmitter.off('data', onDataListener) @@ -68,8 +65,15 @@ export function useNotifyService({ dispatch(notifyUnsubscribeAction(hostname, topic)) } } + } else { + if (hostname == null) { + console.error( + 'NotifyService expected hostname, received null for topic:', + topic + ) + } } - }, [topic]) + }, [topic, host]) return { isNotifyError: isNotifyError.current } diff --git a/react-api-client/src/runs/useAllRunsQuery.ts b/react-api-client/src/runs/useAllRunsQuery.ts index 96a2a1ae456..b02c032928a 100644 --- a/react-api-client/src/runs/useAllRunsQuery.ts +++ b/react-api-client/src/runs/useAllRunsQuery.ts @@ -13,6 +13,10 @@ export type UseAllRunsQueryOptions = UseQueryOptions< Array > +/** + * @property {HostConfig | null | undefined} hostOverride: + * When using all runs query outside of the host context provider, we must specify the host manually. + */ export function useAllRunsQuery( params: GetRunsParams = {}, options: UseAllRunsQueryOptions = {}, diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 05abf3a3d14..be62c7b704f 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -112,6 +112,11 @@ async def create( EngineConflictError: There is a currently active run that cannot be superceded by this new run. """ + await self._runs_publisher.begin_polling_engine_store( + get_current_command=self.get_current_command, + get_state_summary=self._get_state_summary, + run_id=run_id, + ) prev_run_id = self._engine_store.current_run_id if prev_run_id is not None: prev_run_result = await self._engine_store.clear() @@ -120,7 +125,6 @@ async def create( summary=prev_run_result.state_summary, commands=prev_run_result.commands, ) - state_summary = await self._engine_store.create( run_id=run_id, labware_offsets=labware_offsets, @@ -132,11 +136,6 @@ async def create( created_at=created_at, protocol_id=protocol.protocol_id if protocol is not None else None, ) - await self._runs_publisher.begin_polling_engine_store( - get_current_command=self.get_current_command, - get_state_summary=self._get_state_summary, - run_id=run_id, - ) return _build_run( run_resource=run_resource, diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index e32b962e6f3..40e59143e76 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -411,6 +411,7 @@ def remove(self, run_id: str) -> None: raise RunNotFoundError(run_id) self._clear_caches() + self._runs_publisher.publish_runs(run_id=run_id) def _run_exists( self, run_id: str, connection: sqlalchemy.engine.Connection diff --git a/robot-server/robot_server/service/notifications/publishers/runs_publisher.py b/robot-server/robot_server/service/notifications/publishers/runs_publisher.py index 1010b9a2fc0..4f490b0fb07 100644 --- a/robot-server/robot_server/service/notifications/publishers/runs_publisher.py +++ b/robot-server/robot_server/service/notifications/publishers/runs_publisher.py @@ -22,6 +22,7 @@ def __init__(self, client: NotificationClient) -> None: self._run_data_manager_polling = asyncio.Event() self._previous_current_command: Union[CurrentCommand, None] = None self._previous_state_summary_status: Union[EngineStatus, None] = None + self._poller: Optional[asyncio.Task[None]] = None # TODO(jh, 2023-02-02): Instead of polling, emit current_commands directly from PE. async def begin_polling_engine_store( @@ -36,18 +37,22 @@ async def begin_polling_engine_store( current_command: The currently executing command, if any. run_id: ID of the current run. """ - asyncio.create_task( - self._poll_engine_store( - get_current_command=get_current_command, - run_id=run_id, - get_state_summary=get_state_summary, + if self._poller is None: + self._poller = asyncio.create_task( + self._poll_engine_store( + get_current_command=get_current_command, + run_id=run_id, + get_state_summary=get_state_summary, + ) ) - ) async def stop_polling_engine_store(self) -> None: """Stops polling the engine store.""" - self._run_data_manager_polling.set() - await self._client.publish_async(topic=Topics.RUNS_CURRENT_COMMAND.value) + if self._poller is not None: + self._run_data_manager_polling.set() + await self._client.publish_async(topic=Topics.RUNS_CURRENT_COMMAND.value) + self._poller.cancel() + self._poller = None def publish_runs(self, run_id: str) -> None: """Publishes the equivalent of GET /runs and GET /runs/:runId. From 18fbbe19f2ba4899751724701ed97bf4942ad57d Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Fri, 9 Feb 2024 16:16:11 -0500 Subject: [PATCH 10/24] fix(api): get_slice should return last executed command if a run failed (#14449) --- .../protocol_engine/state/commands.py | 11 +++++ .../state/test_command_store.py | 12 +++++ .../state/test_command_view.py | 37 +++++++++++++++- .../test_json_v6_protocol_run.tavern.yaml | 44 +++++++++++++++++++ .../runs/test_json_v6_run_failure.tavern.yaml | 37 +--------------- .../runs/test_papi_v2_run_failure.tavern.yaml | 33 +------------- 6 files changed, 105 insertions(+), 69 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 97c15345721..1c47986c62b 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -152,6 +152,9 @@ class CommandState: are stored on the individual commands themselves. """ + failed_command: Optional[CommandEntry] + """The command, if any, that made the run fail and the index in the command list.""" + finish_error: Optional[ErrorOccurrence] """The error that happened during the post-run finish steps (homing & dropping tips), if any.""" @@ -189,6 +192,7 @@ def __init__( commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_completed_at=None, run_started_at=None, latest_command_hash=None, @@ -281,6 +285,7 @@ def handle_action(self, action: Action) -> None: # noqa: C901 ), ) + self._state.failed_command = self._state.commands_by_id[action.command_id] if prev_entry.command.intent == CommandIntent.SETUP: other_command_ids_to_fail = [ *[i for i in self._state.queued_setup_command_ids], @@ -464,6 +469,12 @@ def get_slice( cursor = commands_by_id[running_command_id].index elif len(queued_command_ids) > 0: cursor = commands_by_id[queued_command_ids.head()].index - 1 + elif ( + self._state.run_result + and self._state.run_result == RunResult.FAILED + and self._state.failed_command + ): + cursor = self._state.failed_command.index else: cursor = total_length - length diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store.py b/api/tests/opentrons/protocol_engine/state/test_command_store.py index a017df3b362..2bce803364d 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_store.py @@ -82,6 +82,7 @@ def test_initial_state( commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, latest_command_hash=None, stopped_by_estop=False, ) @@ -672,6 +673,7 @@ def test_command_store_handles_pause_action(pause_source: PauseSource) -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, latest_command_hash=None, stopped_by_estop=False, ) @@ -699,6 +701,7 @@ def test_command_store_handles_play_action(pause_source: PauseSource) -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=datetime(year=2021, month=1, day=1), latest_command_hash=None, stopped_by_estop=False, @@ -728,6 +731,7 @@ def test_command_store_handles_finish_action() -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=datetime(year=2021, month=1, day=1), latest_command_hash=None, stopped_by_estop=False, @@ -772,6 +776,7 @@ def test_command_store_handles_stop_action(from_estop: bool) -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=datetime(year=2021, month=1, day=1), latest_command_hash=None, stopped_by_estop=from_estop, @@ -800,6 +805,7 @@ def test_command_store_cannot_restart_after_should_stop() -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=None, latest_command_hash=None, stopped_by_estop=False, @@ -930,6 +936,7 @@ def test_command_store_wraps_unknown_errors() -> None: }, ), run_started_at=None, + failed_command=None, latest_command_hash=None, stopped_by_estop=False, ) @@ -989,6 +996,7 @@ def __init__(self, message: str) -> None: detail="yikes", errorCode=ErrorCodes.PIPETTE_NOT_PRESENT.value.code, ), + failed_command=None, run_started_at=None, latest_command_hash=None, stopped_by_estop=False, @@ -1019,6 +1027,7 @@ def test_command_store_ignores_stop_after_graceful_finish() -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=datetime(year=2021, month=1, day=1), latest_command_hash=None, stopped_by_estop=False, @@ -1049,6 +1058,7 @@ def test_command_store_ignores_finish_after_non_graceful_stop() -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=datetime(year=2021, month=1, day=1), latest_command_hash=None, stopped_by_estop=False, @@ -1098,6 +1108,7 @@ def test_command_store_handles_command_failed() -> None: }, run_error=None, finish_error=None, + failed_command=CommandEntry(index=0, command=expected_failed_command), run_started_at=None, latest_command_hash=None, stopped_by_estop=False, @@ -1124,6 +1135,7 @@ def test_handles_hardware_stopped() -> None: commands_by_id=OrderedDict(), run_error=None, finish_error=None, + failed_command=None, run_started_at=None, latest_command_hash=None, stopped_by_estop=False, diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view.py b/api/tests/opentrons/protocol_engine/state/test_command_view.py index 46f431e8c63..82fb21dc1f1 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view.py @@ -24,7 +24,9 @@ RunResult, QueueStatus, ) -from opentrons.protocol_engine.errors import ProtocolCommandFailedError +from opentrons.protocol_engine.errors import ProtocolCommandFailedError, ErrorOccurrence + +from opentrons_shared_data.errors.codes import ErrorCodes from .command_fixtures import ( create_queued_command, @@ -44,6 +46,7 @@ def get_command_view( queued_command_ids: Sequence[str] = (), queued_setup_command_ids: Sequence[str] = (), run_error: Optional[errors.ErrorOccurrence] = None, + failed_command: Optional[CommandEntry] = None, finish_error: Optional[errors.ErrorOccurrence] = None, commands: Sequence[cmd.Command] = (), latest_command_hash: Optional[str] = None, @@ -65,6 +68,7 @@ def get_command_view( queued_setup_command_ids=OrderedSet(queued_setup_command_ids), run_error=run_error, finish_error=finish_error, + failed_command=failed_command, all_command_ids=all_command_ids, commands_by_id=commands_by_id, run_started_at=run_started_at, @@ -793,6 +797,37 @@ def test_get_slice_default_cursor_no_current() -> None: ) +def test_get_slice_default_cursor_failed_command() -> None: + """It should return a slice from the last executed command.""" + command_1 = create_failed_command(command_id="command-id-1") + command_2 = create_failed_command(command_id="command-id-2") + command_3 = create_failed_command( + command_id="command-id-3", + error=ErrorOccurrence( + id="error-id", + errorType="ProtocolEngineError", + createdAt=datetime(year=2022, month=2, day=2), + detail="oh no", + errorCode=ErrorCodes.GENERAL_ERROR.value.code, + ), + ) + command_4 = create_failed_command(command_id="command-id-4") + + subject = get_command_view( + commands=[command_1, command_2, command_3, command_4], + run_result=RunResult.FAILED, + failed_command=CommandEntry(index=2, command=command_3), + ) + + result = subject.get_slice(cursor=None, length=3) + + assert result == CommandSlice( + commands=[command_3, command_4], + cursor=2, + total_length=4, + ) + + def test_get_slice_default_cursor_running() -> None: """It should select a cursor based on the running command, if present.""" command_1 = create_succeeded_command(command_id="command-id-1") diff --git a/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml index 35d815286dc..e468c8de84a 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml @@ -555,3 +555,47 @@ stages: completedAt: '{setup_command_completed_at}' status: succeeded params: {} + + - name: Verify commands succeeded with pageLength and cursor + request: + url: '{ot2_server_base_url}/runs/{run_id}/commands?cursor=5&pageLength=2' + method: GET + response: + status_code: 200 + json: + links: + current: !anydict + meta: + cursor: 5 + totalLength: 15 + data: + - id: !anystr + key: !anystr + commandType: loadLabware + createdAt: !anystr + startedAt: !anystr + completedAt: !anystr + status: succeeded + params: + location: + moduleId: magneticModuleId + loadName: foo_8_plate_33ul + namespace: example + version: 1 + labwareId: destPlateId + displayName: Sample Collection Plate + - id: !anystr + key: !anystr + commandType: loadLabware + createdAt: !anystr + startedAt: !anystr + completedAt: !anystr + status: succeeded + params: + location: + slotName: '8' + loadName: opentrons_96_tiprack_10ul + namespace: opentrons + version: 1 + labwareId: tipRackId + displayName: Opentrons 96 Tip Rack 10 µL \ No newline at end of file diff --git a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml index b5796d80d23..db35113b5ca 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml @@ -83,44 +83,9 @@ stages: links: current: !anydict meta: - cursor: 0 + cursor: 3 totalLength: 5 data: - # Initial home - - id: !anystr - key: !anystr - commandType: home - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: {} - - id: !anystr - key: !anystr - commandType: loadLabware - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: - location: - slotName: '8' - loadName: fixture_1_tiprack_10ul - namespace: fixture - version: 1 - labwareId: tipRackId - displayName: Tip Rack - - id: !anystr - key: !anystr - commandType: loadPipette - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: - pipetteName: p10_single - mount: left - pipetteId: pipetteId - id: !anystr key: !anystr commandType: aspirate diff --git a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml index 14fbb483048..443767c27fc 100644 --- a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml @@ -84,40 +84,9 @@ stages: links: current: !anydict meta: - cursor: 0 + cursor: 3 totalLength: 4 data: - - id: !anystr - key: !anystr - commandType: home - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: {} - - id: !anystr - key: !anystr - commandType: loadLabware - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: - location: - slotName: '1' - loadName: opentrons_96_tiprack_300ul - namespace: opentrons - version: 1 - - id: !anystr - key: !anystr - commandType: loadPipette - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - status: succeeded - params: - pipetteName: p300_single - mount: right - id: !anystr key: !anystr commandType: aspirate From 99edea180cbb68820dc7a561def6826948fa9a7f Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 9 Feb 2024 16:30:05 -0500 Subject: [PATCH 11/24] test(robot-server): Check the number of returned commands in persistence snapshot tests (#14466) --- .../persistence/test_compatibility.py | 107 ++++++++++++------ 1 file changed, 71 insertions(+), 36 deletions(-) diff --git a/robot-server/tests/integration/http_api/persistence/test_compatibility.py b/robot-server/tests/integration/http_api/persistence/test_compatibility.py index f0339c714ac..34458acb97f 100644 --- a/robot-server/tests/integration/http_api/persistence/test_compatibility.py +++ b/robot-server/tests/integration/http_api/persistence/test_compatibility.py @@ -24,15 +24,20 @@ _PORT = "15555" +@dataclass +class Run: + id: str + expected_command_count: int + + @dataclass class Snapshot: """Model to describe a snapshot of a persistence directory.""" version: str expected_protocol_count: int - expected_run_count: int + expected_runs: List[Run] protocols_with_no_analyses: List[str] = field(default_factory=list) - runs_with_no_commands: List[str] = field(default_factory=list) def get_copy(self) -> Path: """Return a path to an isolated copy of this snapshot. @@ -49,30 +54,63 @@ def get_copy(self) -> Path: flex_dev_compat_snapshot = Snapshot( version="ot3_v0.14.0_python_validation", expected_protocol_count=1, - expected_run_count=1, + expected_runs=[Run("305b0cca-fc78-4853-b113-40ac4c30cd8f", 1)], ) snapshots: List[(Snapshot)] = [ - Snapshot(version="v6.0.1", expected_protocol_count=4, expected_run_count=5), - Snapshot(version="v6.1.0", expected_protocol_count=2, expected_run_count=2), - Snapshot(version="v6.2.0", expected_protocol_count=2, expected_run_count=2), + Snapshot( + version="v6.0.1", + expected_protocol_count=4, + expected_runs=[ + Run("7bc1f20d-3925-4aa2-b200-82906112816f", 23), + Run("1b00190c-013f-463d-b371-5bf49b6ad61f", 16), + Run("8165be3f-382f-4b1f-97d7-f3c4ae613868", 65), + Run("467761f3-7339-4b8d-9007-4482500657da", 65), + Run("f7817fa9-bc80-45c0-afea-f7c4af30a663", 333), + ], + ), + Snapshot( + version="v6.1.0", + expected_protocol_count=2, + expected_runs=[ + Run("a4338d46-96af-4e23-877d-1d79227a0946", 147), + Run("efc7374f-2e64-45ea-83fe-bd7a55f2699e", 205), + ], + ), + Snapshot( + version="v6.2.0", + expected_protocol_count=2, + expected_runs=[ + Run("199b991d-db3c-49ff-9b4f-905118c10685", 125), + Run("25a66ec6-2137-4680-8a94-d53c0e2a7488", 87), + ], + ), Snapshot( version="v6.2.0_large", expected_protocol_count=17, - expected_run_count=16, + expected_runs=[ + Run("eeb17dc0-1878-432a-bf3f-33e7d3023b8d", 218), + Run("917cf0f8-8b79-47ab-a407-918c182eb6df", 125), + Run("7b87bac2-680a-4757-a10f-8341a6dce540", 185), + Run("0b97477c-844d-406a-87e8-0852421d7212", 0), + Run("f31659a6-33c9-406d-beb5-da2ec19ef063", 120), + Run("965b45f4-f296-44bf-ae20-df297d3a35af", 8), + Run("b97b0ee8-2ba4-43cd-99aa-601b60f5b75d", 13), + Run("7dd90a28-14b6-4e6f-86a8-41ca6e6e42ae", 11), + Run("dc9162c2-f9f6-48aa-a923-7ba252d3eb1d", 15), + Run("2d9b6f1b-e2fd-40a9-9219-504df2c89305", 0), + Run("9ba966c6-bc2f-4c65-b898-59a4f2530f35", 0), + Run("5f30a0dd-e4da-4f24-abce-7468067d883a", 0), + Run("83f0bad0-6bb2-4ecd-bccf-f14667298168", 0), + Run("0b97363d-0910-43a0-b5a2-b6a62ad2fa6b", 96), + Run("35c014ec-b6ea-4665-8149-5c6340cbc5ca", 0), + Run("d2b68ac6-5c4f-4914-bc2e-f306a976d582", 220), + ], protocols_with_no_analyses=[ "429e72e1-6ff1-4328-8a1d-c13fe3ac0c80", "e3515d46-3c3b-425b-8734-bd6e38d6a729", ], - runs_with_no_commands=[ - "0b97477c-844d-406a-87e8-0852421d7212", - "2d9b6f1b-e2fd-40a9-9219-504df2c89305", - "9ba966c6-bc2f-4c65-b898-59a4f2530f35", - "5f30a0dd-e4da-4f24-abce-7468067d883a", - "83f0bad0-6bb2-4ecd-bccf-f14667298168", - "35c014ec-b6ea-4665-8149-5c6340cbc5ca", - ], ), flex_dev_compat_snapshot, ] @@ -133,27 +171,24 @@ async def test_protocols_analyses_and_runs_available_from_older_persistence_dir( else: assert number_of_analyses > 0 - all_runs = (await robot_client.get_runs()).json() - all_run_ids = [r["id"] for r in all_runs["data"]] - assert len(all_run_ids) == snapshot.expected_run_count - - for run_id in all_run_ids: - await robot_client.get_run(run_id=run_id) - - all_command_summaries = ( - await robot_client.get_run_commands( - run_id=run_id, - page_length=999999, # Big enough to include all commands. - ) - ).json() - - if run_id in snapshot.runs_with_no_commands: - assert len(all_command_summaries["data"]) == 0 - else: - assert len(all_command_summaries["data"]) > 0 - # Ideally, we would also fetch full commands via - # `GET /runs/{run_id}/commands/{command_id}`. - # We skip it for performance. Adds ~10+ seconds + all_runs = (await robot_client.get_runs()).json() + all_run_ids = [r["id"] for r in all_runs["data"]] + assert all_run_ids == [r.id for r in snapshot.expected_runs] + + for expected_run in snapshot.expected_runs: + await robot_client.get_run(run_id=expected_run.id) + + all_command_summaries = ( + await robot_client.get_run_commands( + run_id=expected_run.id, + page_length=999999, # Big enough to include all commands. + ) + ).json()["data"] + + assert len(all_command_summaries) == expected_run.expected_command_count + # Ideally, we would also fetch full commands via + # `GET /runs/{run_id}/commands/{command_id}`. + # We skip it for performance. Adds ~10+ seconds # TODO(mm, 2023-08-12): We can remove this test when we remove special handling for these From 180d9cad46021413811331ca85e69faa88605f01 Mon Sep 17 00:00:00 2001 From: Sarah Breen Date: Fri, 9 Feb 2024 17:21:05 -0500 Subject: [PATCH 12/24] fix(app): ensure protocol doesn't have stale status before analysis completion (#14451) fix RQA-2297 --- app/src/organisms/ProtocolsLanding/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/organisms/ProtocolsLanding/utils.ts b/app/src/organisms/ProtocolsLanding/utils.ts index 4eb061fdae9..59ccfc2e852 100644 --- a/app/src/organisms/ProtocolsLanding/utils.ts +++ b/app/src/organisms/ProtocolsLanding/utils.ts @@ -10,7 +10,7 @@ export function getAnalysisStatus( ): AnalysisStatus { if (isAnalyzing) { return 'loading' - } else if (analysis?.liquids == null) { + } else if (analysis != null && analysis?.liquids == null) { return 'stale' } else if (analysis != null) { return analysis.errors.length > 0 ? 'error' : 'complete' From 0c1956423db593631862414645b3e5652de3bac5 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:26:52 -0500 Subject: [PATCH 13/24] refactor(app): add robot serial number to mixpanel analytics (#14436) * refactor(app): add robot serial number to mixpanel analytics closes RAUT-899 --- .../ChooseProtocolSlideout/index.tsx | 3 +- .../index.tsx | 5 ++- .../HistoricalProtocolRunOverflowMenu.tsx | 2 +- .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 5 ++- .../__tests__/ProtocolRunHeader.test.tsx | 8 ++-- ...HistoricalProtocolRunOverflowMenu.test.tsx | 8 ++-- .../useProtocolRunAnalyticsData.test.tsx | 30 ++++++++++----- .../__tests__/useRobotAnalyticsData.test.tsx | 17 ++++++++- .../useTrackCreateProtocolRunEvent.test.tsx | 6 +-- .../useTrackProtocolRunEvent.test.tsx | 38 ++++++++++++------- .../hooks/useProtocolRunAnalyticsData.ts | 21 ++++++++-- .../Devices/hooks/useRobotAnalyticsData.ts | 4 ++ .../hooks/useTrackCreateProtocolRunEvent.ts | 6 ++- .../Devices/hooks/useTrackProtocolRunEvent.ts | 8 +++- .../__tests__/RecentRunProtocolCard.test.tsx | 9 +++-- .../RunningProtocol/ConfirmCancelRunModal.tsx | 6 ++- .../__tests__/ConfirmCancelRunModal.test.tsx | 19 ++++++++-- .../RunDetails/ConfirmCancelModal.tsx | 5 ++- .../__tests__/ConfirmCancelModal.test.tsx | 11 ++++-- .../__tests__/ProtocolSetup.test.tsx | 2 +- app/src/pages/ProtocolSetup/index.tsx | 2 +- app/src/pages/RunSummary/index.tsx | 6 +-- .../__tests__/RunningProtocol.test.tsx | 18 +++++++-- app/src/pages/RunningProtocol/index.tsx | 2 +- app/src/redux/analytics/types.ts | 2 + 25 files changed, 175 insertions(+), 68 deletions(-) diff --git a/app/src/organisms/ChooseProtocolSlideout/index.tsx b/app/src/organisms/ChooseProtocolSlideout/index.tsx index 2b928495a7b..e1dada9d64a 100644 --- a/app/src/organisms/ChooseProtocolSlideout/index.tsx +++ b/app/src/organisms/ChooseProtocolSlideout/index.tsx @@ -82,7 +82,8 @@ export function ChooseProtocolSlideoutComponent( : [] const { trackCreateProtocolRunEvent } = useTrackCreateProtocolRunEvent( - selectedProtocol + selectedProtocol, + name ) const { diff --git a/app/src/organisms/ChooseRobotToRunProtocolSlideout/index.tsx b/app/src/organisms/ChooseRobotToRunProtocolSlideout/index.tsx index c4e50fc5a88..b2215914088 100644 --- a/app/src/organisms/ChooseRobotToRunProtocolSlideout/index.tsx +++ b/app/src/organisms/ChooseRobotToRunProtocolSlideout/index.tsx @@ -49,11 +49,12 @@ export function ChooseRobotToRunProtocolSlideoutComponent( mostRecentAnalysis, } = storedProtocolData + const [selectedRobot, setSelectedRobot] = React.useState(null) const { trackCreateProtocolRunEvent } = useTrackCreateProtocolRunEvent( - storedProtocolData + storedProtocolData, + selectedRobot?.name ?? '' ) - const [selectedRobot, setSelectedRobot] = React.useState(null) const offsetCandidates = useOffsetCandidatesForAnalysis( mostRecentAnalysis, selectedRobot?.ip ?? null diff --git a/app/src/organisms/Devices/HistoricalProtocolRunOverflowMenu.tsx b/app/src/organisms/Devices/HistoricalProtocolRunOverflowMenu.tsx index 440c371ec53..bf06e0db263 100644 --- a/app/src/organisms/Devices/HistoricalProtocolRunOverflowMenu.tsx +++ b/app/src/organisms/Devices/HistoricalProtocolRunOverflowMenu.tsx @@ -129,7 +129,7 @@ function MenuDropdown(props: MenuDropdownProps): JSX.Element { closeOverflowMenu(e) } const trackEvent = useTrackEvent() - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const { reset } = useRunControls(runId, onResetSuccess) const { deleteRun } = useDeleteRunMutation() diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index d12da9838cf..e299657fe05 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -155,7 +155,7 @@ export function ProtocolRunHeader({ isProtocolAnalyzing, } = useProtocolDetailsForRun(runId) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const robotAnalyticsData = useRobotAnalyticsData(robotName) const isRobotViewable = useIsRobotViewable(robotName) const runStatus = useRunStatus(runId) @@ -453,6 +453,7 @@ export function ProtocolRunHeader({ setShowConfirmCancelModal(false)} runId={runId} + robotName={robotName} /> ) : null} {showDropTipWizard && @@ -575,7 +576,7 @@ function ActionButton(props: ActionButtonProps): JSX.Element { enabled: runStatus != null && START_RUN_STATUSES.includes(runStatus), })?.data?.data ?? [] const trackEvent = useTrackEvent() - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const [targetProps, tooltipProps] = useHoverTooltip() const { play, diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index 5453af0efd9..2b0a2813660 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -406,9 +406,11 @@ describe('ProtocolRunHeader', () => { when(mockUseProtocolDetailsForRun) .calledWith(RUN_ID) .mockReturnValue(PROTOCOL_DETAILS) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) when(mockUseDismissCurrentRunMutation) .calledWith() .mockReturnValue({ diff --git a/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx b/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx index 0939700387e..12461640384 100644 --- a/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx +++ b/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx @@ -106,9 +106,11 @@ describe('HistoricalProtocolRunOverflowMenu', () => { deleteRun: jest.fn(), } as any) ) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) when(mockUseRunControls) .calledWith(RUN_ID, expect.anything()) .mockReturnValue({ diff --git a/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx index 0cfbb7fce93..554a1952c66 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx @@ -45,6 +45,7 @@ let store: Store = createStore(jest.fn(), {}) const RUN_ID = '1' const RUN_ID_2 = '2' +const ROBOT_NAME = 'otie' const PIPETTES = [ { id: '1', pipetteName: 'testModelLeft' }, @@ -111,9 +112,12 @@ describe('useProtocolAnalysisErrors hook', () => { }) it('returns getProtocolRunAnalyticsData function', () => { - const { result } = renderHook(() => useProtocolRunAnalyticsData(RUN_ID), { - wrapper, - }) + const { result } = renderHook( + () => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME), + { + wrapper, + } + ) expect(typeof result.current.getProtocolRunAnalyticsData).toEqual( 'function' ) @@ -123,9 +127,12 @@ describe('useProtocolAnalysisErrors hook', () => { when(mockUseProtocolDetailsForRun) .calledWith(RUN_ID_2) .mockReturnValue({ protocolData: ROBOT_PROTOCOL_ANALYSIS } as any) - const { result } = renderHook(() => useProtocolRunAnalyticsData(RUN_ID_2), { - wrapper, - }) + const { result } = renderHook( + () => useProtocolRunAnalyticsData(RUN_ID_2, ROBOT_NAME), + { + wrapper, + } + ) const protocolRunAnalyticsData = await waitFor(() => result.current.getProtocolRunAnalyticsData() ) @@ -142,15 +149,19 @@ describe('useProtocolAnalysisErrors hook', () => { protocolText: 'hashedString', protocolType: '', robotType: 'OT-2 Standard', + robotSerialNumber: '', }, runTime: '1:00:00', }) }) it('getProtocolRunAnalyticsData returns fallback stored data when robot data unavailable', async () => { - const { result } = renderHook(() => useProtocolRunAnalyticsData(RUN_ID), { - wrapper, - }) + const { result } = renderHook( + () => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME), + { + wrapper, + } + ) const protocolRunAnalyticsData = await waitFor(() => result.current.getProtocolRunAnalyticsData() ) @@ -167,6 +178,7 @@ describe('useProtocolAnalysisErrors hook', () => { protocolText: 'hashedString', protocolType: 'json', robotType: 'OT-2 Standard', + robotSerialNumber: '', }, runTime: '1:00:00', }) diff --git a/app/src/organisms/Devices/hooks/__tests__/useRobotAnalyticsData.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useRobotAnalyticsData.test.tsx index 9719675a9f4..ead00dac63f 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useRobotAnalyticsData.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useRobotAnalyticsData.test.tsx @@ -9,9 +9,12 @@ import { useRobot } from '../' import { useRobotAnalyticsData } from '../useRobotAnalyticsData' import { getAttachedPipettes } from '../../../../redux/pipettes' import { getRobotSettings } from '../../../../redux/robot-settings' +import { mockConnectableRobot } from '../../../../redux/discovery/__fixtures__' + import { getRobotApiVersion, getRobotFirmwareVersion, + getRobotSerialNumber, } from '../../../../redux/discovery' import type { DiscoveredRobot } from '../../../../redux/discovery/types' @@ -36,6 +39,9 @@ const mockGetRobotFirmwareVersion = getRobotFirmwareVersion as jest.MockedFuncti const mockGetAttachedPipettes = getAttachedPipettes as jest.MockedFunction< typeof getAttachedPipettes > +const mockGetRobotSerialNumber = getRobotSerialNumber as jest.MockedFunction< + typeof getRobotSerialNumber +> const ROBOT_SETTINGS = [ { id: `setting1`, value: true, title: '', description: '' }, @@ -47,6 +53,7 @@ const ATTACHED_PIPETTES = { left: { id: '1', model: 'testModelLeft' }, right: { id: '2', model: 'testModelRight' }, } +const ROBOT_SERIAL_NUMBER = 'OT123' let wrapper: React.FunctionComponent<{ children: React.ReactNode }> let store: Store = createStore(jest.fn(), {}) @@ -70,6 +77,7 @@ describe('useProtocolAnalysisErrors hook', () => { mockGetAttachedPipettes.mockReturnValue( ATTACHED_PIPETTES as AttachedPipettesByMount ) + mockGetRobotSerialNumber.mockReturnValue(ROBOT_SERIAL_NUMBER) }) afterEach(() => { @@ -87,7 +95,13 @@ describe('useProtocolAnalysisErrors hook', () => { it('returns robot analytics data when robot exists', () => { when(mockUseRobot) .calledWith('otie') - .mockReturnValue({} as DiscoveredRobot) + .mockReturnValue({ + ...mockConnectableRobot, + health: { + ...mockConnectableRobot.health, + robot_serial: ROBOT_SERIAL_NUMBER, + }, + } as DiscoveredRobot) const { result } = renderHook(() => useRobotAnalyticsData('otie'), { wrapper, @@ -99,6 +113,7 @@ describe('useProtocolAnalysisErrors hook', () => { robotLeftPipette: 'testModelLeft', robotRightPipette: 'testModelRight', robotSmoothieVersion: ROBOT_FIRMWARE_VERSION, + robotSerialNumber: ROBOT_SERIAL_NUMBER, }) }) }) diff --git a/app/src/organisms/Devices/hooks/__tests__/useTrackCreateProtocolRunEvent.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useTrackCreateProtocolRunEvent.test.tsx index de223800946..bf9969b5a7b 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useTrackCreateProtocolRunEvent.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useTrackCreateProtocolRunEvent.test.tsx @@ -72,7 +72,7 @@ describe('useTrackCreateProtocolRunEvent hook', () => { it('returns trackCreateProtocolRunEvent function', () => { const { result } = renderHook( - () => useTrackCreateProtocolRunEvent(storedProtocolData), + () => useTrackCreateProtocolRunEvent(storedProtocolData, 'otie'), { wrapper, } @@ -82,7 +82,7 @@ describe('useTrackCreateProtocolRunEvent hook', () => { it('trackCreateProtocolRunEvent invokes trackEvent with correct props', async () => { const { result } = renderHook( - () => useTrackCreateProtocolRunEvent(storedProtocolData), + () => useTrackCreateProtocolRunEvent(storedProtocolData, 'otie'), { wrapper, } @@ -107,7 +107,7 @@ describe('useTrackCreateProtocolRunEvent hook', () => { }) ) const { result } = renderHook( - () => useTrackCreateProtocolRunEvent(storedProtocolData), + () => useTrackCreateProtocolRunEvent(storedProtocolData, 'otie'), { wrapper, } diff --git a/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx index 5dfff1c91b4..3adff13e65c 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx @@ -27,6 +27,7 @@ const mockUseProtocolRunAnalyticsData = useProtocolRunAnalyticsData as jest.Mock > const RUN_ID = 'runId' +const ROBOT_NAME = 'otie' const PROTOCOL_PROPERTIES = { protocolType: 'python' } let mockTrackEvent: jest.Mock @@ -54,9 +55,11 @@ describe('useTrackProtocolRunEvent hook', () => { ) ) mockUseTrackEvent.mockReturnValue(mockTrackEvent) - when(mockUseProtocolRunAnalyticsData).calledWith(RUN_ID).mockReturnValue({ - getProtocolRunAnalyticsData: mockGetProtocolRunAnalyticsData, - }) + when(mockUseProtocolRunAnalyticsData) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + getProtocolRunAnalyticsData: mockGetProtocolRunAnalyticsData, + }) }) afterEach(() => { @@ -65,16 +68,22 @@ describe('useTrackProtocolRunEvent hook', () => { }) it('returns trackProtocolRunEvent function', () => { - const { result } = renderHook(() => useTrackProtocolRunEvent(RUN_ID), { - wrapper, - }) + const { result } = renderHook( + () => useTrackProtocolRunEvent(RUN_ID, ROBOT_NAME), + { + wrapper, + } + ) expect(typeof result.current.trackProtocolRunEvent).toBe('function') }) it('trackProtocolRunEvent invokes trackEvent with correct props', async () => { - const { result } = renderHook(() => useTrackProtocolRunEvent(RUN_ID), { - wrapper, - }) + const { result } = renderHook( + () => useTrackProtocolRunEvent(RUN_ID, ROBOT_NAME), + { + wrapper, + } + ) await waitFor(() => result.current.trackProtocolRunEvent({ name: ANALYTICS_PROTOCOL_RUN_START, @@ -89,16 +98,19 @@ describe('useTrackProtocolRunEvent hook', () => { it('trackProtocolRunEvent calls trackEvent without props when error is thrown in getProtocolRunAnalyticsData', async () => { when(mockUseProtocolRunAnalyticsData) - .calledWith('errorId') + .calledWith('errorId', ROBOT_NAME) .mockReturnValue({ getProtocolRunAnalyticsData: () => new Promise(() => { throw new Error('error') }), }) - const { result } = renderHook(() => useTrackProtocolRunEvent('errorId'), { - wrapper, - }) + const { result } = renderHook( + () => useTrackProtocolRunEvent('errorId', ROBOT_NAME), + { + wrapper, + } + ) await waitFor(() => result.current.trackProtocolRunEvent({ name: ANALYTICS_PROTOCOL_RUN_START, diff --git a/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts b/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts index 653e352384c..a6b83a93088 100644 --- a/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts +++ b/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts @@ -2,7 +2,12 @@ import { useSelector } from 'react-redux' import { hash } from '../../../redux/analytics/hash' import { getStoredProtocol } from '../../../redux/protocol-storage' -import { useStoredProtocolAnalysis, useProtocolDetailsForRun } from './' +import { getRobotSerialNumber } from '../../../redux/discovery' +import { + useRobot, + useStoredProtocolAnalysis, + useProtocolDetailsForRun, +} from './' import { useProtocolMetadata } from './useProtocolMetadata' import { useRunTimestamps } from '../../RunTimeControl/hooks' import { formatInterval } from '../../RunTimeControl/utils' @@ -16,13 +21,18 @@ import type { State } from '../../../redux/types' export const parseProtocolRunAnalyticsData = ( protocolAnalysis: ProtocolAnalysisOutput | null, storedProtocol: StoredProtocolData | null, - startedAt: string | null + startedAt: string | null, + robotName: string ) => () => { const hashTasks = [ hash(protocolAnalysis?.metadata?.author) ?? '', hash(storedProtocol?.srcFiles?.toString() ?? '') ?? '', ] + const robot = useRobot(robotName) + const serialNumber = + robot?.status != null ? getRobotSerialNumber(robot) : null + return Promise.all(hashTasks).then(([protocolAuthor, protocolText]) => ({ protocolRunAnalyticsData: { protocolType: protocolAnalysis?.config?.protocolType ?? '', @@ -49,6 +59,7 @@ export const parseProtocolRunAnalyticsData = ( protocolAnalysis?.robotType != null ? protocolAnalysis?.robotType : storedProtocol?.mostRecentAnalysis?.robotType, + robotSerialNumber: serialNumber ?? '', }, runTime: startedAt != null ? formatInterval(startedAt, Date()) : EMPTY_TIMESTAMP, @@ -68,7 +79,8 @@ type GetProtocolRunAnalyticsData = () => Promise<{ * data properties for use in event trackEvent */ export function useProtocolRunAnalyticsData( - runId: string | null + runId: string | null, + robotName: string ): { getProtocolRunAnalyticsData: GetProtocolRunAnalyticsData } { @@ -96,7 +108,8 @@ export function useProtocolRunAnalyticsData( const getProtocolRunAnalyticsData = parseProtocolRunAnalyticsData( protocolAnalysis as ProtocolAnalysisOutput | null, storedProtocol, - startedAt + startedAt, + robotName ) return { getProtocolRunAnalyticsData } diff --git a/app/src/organisms/Devices/hooks/useRobotAnalyticsData.ts b/app/src/organisms/Devices/hooks/useRobotAnalyticsData.ts index 6ca8c95fbca..94037f05b2a 100644 --- a/app/src/organisms/Devices/hooks/useRobotAnalyticsData.ts +++ b/app/src/organisms/Devices/hooks/useRobotAnalyticsData.ts @@ -7,6 +7,7 @@ import { getRobotSettings, fetchSettings } from '../../../redux/robot-settings' import { getRobotApiVersion, getRobotFirmwareVersion, + getRobotSerialNumber, } from '../../../redux/discovery' import type { State, Dispatch } from '../../../redux/types' @@ -30,6 +31,8 @@ export function useRobotAnalyticsData( const settings = useSelector((state: State) => getRobotSettings(state, robotName) ) + const serialNumber = + robot?.status != null ? getRobotSerialNumber(robot) : null const dispatch = useDispatch() React.useEffect(() => { @@ -50,6 +53,7 @@ export function useRobotAnalyticsData( robotSmoothieVersion: getRobotFirmwareVersion(robot) ?? '', robotLeftPipette: pipettes.left?.model ?? '', robotRightPipette: pipettes.right?.model ?? '', + robotSerialNumber: serialNumber ?? '', } ) } diff --git a/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts b/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts index f698b08e0f2..400f93fac40 100644 --- a/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts +++ b/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts @@ -18,7 +18,8 @@ type TrackCreateProtocolRunEvent = ( ) => void export function useTrackCreateProtocolRunEvent( - protocol: StoredProtocolData | null + protocol: StoredProtocolData | null, + robotName: string ): { trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent } { const trackEvent = useTrackEvent() @@ -29,7 +30,8 @@ export function useTrackCreateProtocolRunEvent( const getProtocolRunAnalyticsData = parseProtocolRunAnalyticsData( storedProtocolAnalysis, protocol, - null + null, + robotName ) const trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent = ({ diff --git a/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts b/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts index 89dc55a0f23..7cc354805b2 100644 --- a/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts +++ b/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts @@ -11,10 +11,14 @@ export type TrackProtocolRunEvent = ( ) => void export function useTrackProtocolRunEvent( - runId: string | null + runId: string | null, + robotName: string ): { trackProtocolRunEvent: TrackProtocolRunEvent } { const trackEvent = useTrackEvent() - const { getProtocolRunAnalyticsData } = useProtocolRunAnalyticsData(runId) + const { getProtocolRunAnalyticsData } = useProtocolRunAnalyticsData( + runId, + robotName + ) const trackProtocolRunEvent: TrackProtocolRunEvent = ({ name, diff --git a/app/src/organisms/OnDeviceDisplay/RobotDashboard/__tests__/RecentRunProtocolCard.test.tsx b/app/src/organisms/OnDeviceDisplay/RobotDashboard/__tests__/RecentRunProtocolCard.test.tsx index 7b76e34aeb4..7c4aa964296 100644 --- a/app/src/organisms/OnDeviceDisplay/RobotDashboard/__tests__/RecentRunProtocolCard.test.tsx +++ b/app/src/organisms/OnDeviceDisplay/RobotDashboard/__tests__/RecentRunProtocolCard.test.tsx @@ -36,6 +36,7 @@ jest.mock('../../../../resources/runs/useNotifyAllRunsQuery') jest.mock('../../../../resources/health/hooks') const RUN_ID = 'mockRunId' +const ROBOT_NAME = 'otie' const mockMissingPipette = [ { @@ -144,9 +145,11 @@ describe('RecentRunProtocolCard', () => { mockUseProtocolQuery.mockReturnValue({ data: { data: { metadata: { protocolName: 'mockProtocol' } } }, } as any) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) mockCloneRun = jest.fn() when(mockUseCloneRun) .calledWith(RUN_ID, expect.anything()) diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx index 6f1c103df0f..21f48b59843 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx @@ -1,6 +1,7 @@ import * as React from 'react' import { useTranslation } from 'react-i18next' import { useHistory } from 'react-router-dom' +import { useSelector } from 'react-redux' import { RUN_STATUS_STOPPED } from '@opentrons/api-client' import { @@ -21,6 +22,7 @@ import { Modal } from '../../../molecules/Modal' import { useTrackProtocolRunEvent } from '../../../organisms/Devices/hooks' import { useRunStatus } from '../../../organisms/RunTimeControl/hooks' import { ANALYTICS_PROTOCOL_RUN_CANCEL } from '../../../redux/analytics' +import { getLocalRobot } from '../../../redux/discovery' import { CancelingRunModal } from './CancelingRunModal' import type { ModalHeaderBaseProps } from '../../../molecules/Modal/types' @@ -45,7 +47,9 @@ export function ConfirmCancelRunModal({ isLoading: isDismissing, } = useDismissCurrentRunMutation() const runStatus = useRunStatus(runId) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const localRobot = useSelector(getLocalRobot) + const robotName = localRobot?.name ?? '' + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const history = useHistory() const [isCanceling, setIsCanceling] = React.useState(false) diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/ConfirmCancelRunModal.test.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/ConfirmCancelRunModal.test.tsx index 3208e81fb63..bd802d535b8 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/ConfirmCancelRunModal.test.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/ConfirmCancelRunModal.test.tsx @@ -14,6 +14,8 @@ import { i18n } from '../../../../i18n' import { useTrackProtocolRunEvent } from '../../../../organisms/Devices/hooks' import { useRunStatus } from '../../../../organisms/RunTimeControl/hooks' import { useTrackEvent } from '../../../../redux/analytics' +import { getLocalRobot } from '../../../../redux/discovery' +import { mockConnectedRobot } from '../../../../redux/discovery/__fixtures__' import { ConfirmCancelRunModal } from '../ConfirmCancelRunModal' import { CancelingRunModal } from '../CancelingRunModal' @@ -23,6 +25,7 @@ jest.mock('../../../../organisms/RunTimeControl/hooks') jest.mock('../../../../redux/analytics') jest.mock('../../../ProtocolUpload/hooks') jest.mock('../CancelingRunModal') +jest.mock('../../../../redux/discovery') const mockPush = jest.fn() let mockStopRun: jest.Mock @@ -56,6 +59,9 @@ const mockCancelingRunModal = CancelingRunModal as jest.MockedFunction< const mockUseRunStatus = useRunStatus as jest.MockedFunction< typeof useRunStatus > +const mockGetLocalRobot = getLocalRobot as jest.MockedFunction< + typeof getLocalRobot +> const render = (props: React.ComponentProps) => { return renderWithProviders( @@ -69,6 +75,7 @@ const render = (props: React.ComponentProps) => { } const RUN_ID = 'mock_runID' +const ROBOT_NAME = 'otie' const mockFn = jest.fn() describe('ConfirmCancelRunModal', () => { @@ -86,15 +93,21 @@ describe('ConfirmCancelRunModal', () => { mockTrackProtocolRunEvent = jest.fn( () => new Promise(resolve => resolve({})) ) + mockGetLocalRobot.mockReturnValue({ + ...mockConnectedRobot, + name: ROBOT_NAME, + }) mockUseStopRunMutation.mockReturnValue({ stopRun: mockStopRun } as any) mockUseDismissCurrentRunMutation.mockReturnValue({ dismissCurrentRun: mockDismissCurrentRun, isLoading: false, } as any) mockUseTrackEvent.mockReturnValue(mockTrackEvent) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) mockCancelingRunModal.mockReturnValue(
mock CancelingRunModal
) when(mockUseRunStatus).calledWith(RUN_ID).mockReturnValue(RUN_STATUS_IDLE) }) diff --git a/app/src/organisms/RunDetails/ConfirmCancelModal.tsx b/app/src/organisms/RunDetails/ConfirmCancelModal.tsx index 5bd04903937..d55d4900e4b 100644 --- a/app/src/organisms/RunDetails/ConfirmCancelModal.tsx +++ b/app/src/organisms/RunDetails/ConfirmCancelModal.tsx @@ -28,16 +28,17 @@ import { ANALYTICS_PROTOCOL_RUN_CANCEL } from '../../redux/analytics' export interface ConfirmCancelModalProps { onClose: () => unknown runId: string + robotName: string } export function ConfirmCancelModal( props: ConfirmCancelModalProps ): JSX.Element { - const { onClose, runId } = props + const { onClose, runId, robotName } = props const { stopRun } = useStopRunMutation() const [isCanceling, setIsCanceling] = React.useState(false) const runStatus = useRunStatus(runId) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const { t } = useTranslation('run_details') const cancelRun: React.MouseEventHandler = (e): void => { diff --git a/app/src/organisms/RunDetails/__tests__/ConfirmCancelModal.test.tsx b/app/src/organisms/RunDetails/__tests__/ConfirmCancelModal.test.tsx index ed73d19abe3..a64c8a88e28 100644 --- a/app/src/organisms/RunDetails/__tests__/ConfirmCancelModal.test.tsx +++ b/app/src/organisms/RunDetails/__tests__/ConfirmCancelModal.test.tsx @@ -41,6 +41,7 @@ const render = (props: React.ComponentProps) => { } const RUN_ID = 'mockRunId' +const ROBOT_NAME = 'otie' let mockStopRun: jest.Mock let mockTrackEvent: jest.Mock let mockTrackProtocolRunEvent: jest.Mock @@ -56,11 +57,13 @@ describe('ConfirmCancelModal', () => { mockUseStopRunMutation.mockReturnValue({ stopRun: mockStopRun } as any) mockUseRunStatus.mockReturnValue(RUN_STATUS_RUNNING) mockUseTrackEvent.mockReturnValue(mockTrackEvent) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) - props = { onClose: jest.fn(), runId: RUN_ID } + props = { onClose: jest.fn(), runId: RUN_ID, robotName: ROBOT_NAME } }) afterEach(() => { diff --git a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx index 196fde2acad..ca6d6b275f0 100644 --- a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx +++ b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx @@ -345,7 +345,7 @@ describe('ProtocolSetup', () => { } as unknown) as any) when(mockUseDeckConfigurationCompatibility).mockReturnValue([]) when(mockUseTrackProtocolRunEvent) - .calledWith(RUN_ID) + .calledWith(RUN_ID, ROBOT_NAME) .mockReturnValue({ trackProtocolRunEvent: mockTrackProtocolRunEvent }) }) diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 46011492e54..950eb60991e 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -343,7 +343,7 @@ function PrepareToRun({ mostRecentAnalysis ) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const robotAnalyticsData = useRobotAnalyticsData(robotName) const requiredDeckConfigCompatibility = getRequiredDeckConfig( diff --git a/app/src/pages/RunSummary/index.tsx b/app/src/pages/RunSummary/index.tsx index 521d1fd0480..272544af63f 100644 --- a/app/src/pages/RunSummary/index.tsx +++ b/app/src/pages/RunSummary/index.tsx @@ -107,12 +107,12 @@ export function RunSummary(): JSX.Element { const [showSplash, setShowSplash] = React.useState( runStatus === RUN_STATUS_FAILED || runStatus === RUN_STATUS_SUCCEEDED ) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) + const localRobot = useSelector(getLocalRobot) + const robotName = localRobot?.name ?? 'no name' + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const { reset } = useRunControls(runId) const trackEvent = useTrackEvent() const { closeCurrentRun, isClosingCurrentRun } = useCloseCurrentRun() - const localRobot = useSelector(getLocalRobot) - const robotName = localRobot?.name ?? 'no name' const robotAnalyticsData = useRobotAnalyticsData(robotName) const [showRunFailedModal, setShowRunFailedModal] = React.useState( false diff --git a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx index 83f4576fd6a..0656e2180f0 100644 --- a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx +++ b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx @@ -17,6 +17,7 @@ import { useRunActionMutations, } from '@opentrons/react-api-client' +import { getLocalRobot } from '../../../redux/discovery' import { mockRobotSideAnalysis } from '../../../organisms/CommandText/__fixtures__' import { CurrentRunningProtocolCommand, @@ -24,6 +25,7 @@ import { RunningProtocolSkeleton, } from '../../../organisms/OnDeviceDisplay/RunningProtocol' import { mockUseAllCommandsResponseNonDeterministic } from '../../../organisms/RunProgressMeter/__fixtures__' +import { mockConnectedRobot } from '../../../redux/discovery/__fixtures__' import { useRunStatus, useRunTimestamps, @@ -100,8 +102,12 @@ const mockOpenDoorAlertModal = OpenDoorAlertModal as jest.MockedFunction< const mockUseNotifyLastRunCommandKey = useNotifyLastRunCommandKey as jest.MockedFunction< typeof useNotifyLastRunCommandKey > +const mockGetLocalRobot = getLocalRobot as jest.MockedFunction< + typeof getLocalRobot +> const RUN_ID = 'run_id' +const ROBOT_NAME = 'otie' const PROTOCOL_ID = 'protocol_id' const PROTOCOL_KEY = 'protocol_key' const PROTOCOL_ANALYSIS = { @@ -160,6 +166,10 @@ describe('RunningProtocol', () => { stoppedAt: '', completedAt: '2022-05-04T18:24:41.833862+00:00', }) + mockGetLocalRobot.mockReturnValue({ + ...mockConnectedRobot, + name: ROBOT_NAME, + }) when(mockUseRunActionMutations).calledWith(RUN_ID).mockReturnValue({ playRun: mockPlayRun, pauseRun: mockPauseRun, @@ -168,9 +178,11 @@ describe('RunningProtocol', () => { isPauseRunActionLoading: false, isStopRunActionLoading: false, }) - when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ - trackProtocolRunEvent: mockTrackProtocolRunEvent, - }) + when(mockUseTrackProtocolRunEvent) + .calledWith(RUN_ID, ROBOT_NAME) + .mockReturnValue({ + trackProtocolRunEvent: mockTrackProtocolRunEvent, + }) when(mockUseMostRecentCompletedAnalysis) .calledWith(RUN_ID) .mockReturnValue(mockRobotSideAnalysis) diff --git a/app/src/pages/RunningProtocol/index.tsx b/app/src/pages/RunningProtocol/index.tsx index 4160cdd2ccc..a702b7bf881 100644 --- a/app/src/pages/RunningProtocol/index.tsx +++ b/app/src/pages/RunningProtocol/index.tsx @@ -110,9 +110,9 @@ export function RunningProtocol(): JSX.Element { protocolRecord?.data.metadata.protocolName ?? protocolRecord?.data.files[0].name const { playRun, pauseRun } = useRunActionMutations(runId) - const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) const localRobot = useSelector(getLocalRobot) const robotName = localRobot != null ? localRobot.name : 'no name' + const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const robotAnalyticsData = useRobotAnalyticsData(robotName) const robotType = useRobotType(robotName) React.useEffect(() => { diff --git a/app/src/redux/analytics/types.ts b/app/src/redux/analytics/types.ts index d5b96a2dd8c..ceb24166d0e 100644 --- a/app/src/redux/analytics/types.ts +++ b/app/src/redux/analytics/types.ts @@ -21,6 +21,7 @@ export interface ProtocolAnalyticsData { protocolText: string pipettes: string modules: string + robotSerialNumber: string } export type RobotAnalyticsData = { @@ -28,6 +29,7 @@ export type RobotAnalyticsData = { robotSmoothieVersion: string robotLeftPipette: string robotRightPipette: string + robotSerialNumber: string } & { // feature flags // e.g. robotFF_settingName From 416d823d177c2deaa464860ddee45e2a2bdfa3eb Mon Sep 17 00:00:00 2001 From: koji Date: Mon, 12 Feb 2024 22:57:33 +0900 Subject: [PATCH 14/24] docs: update nodejs version in the doc for 7.2.0 (#14471) * docs: update nodejs version in the doc --- DEV_SETUP.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/DEV_SETUP.md b/DEV_SETUP.md index d6522e65d25..d0035baaf84 100644 --- a/DEV_SETUP.md +++ b/DEV_SETUP.md @@ -13,7 +13,7 @@ You will need the following tools installed to develop on the Opentrons platform - curl - ssh - Python v3.10 -- Node.js v16 +- Node.js v18 ### macOS @@ -82,10 +82,10 @@ Close and re-open your terminal to confirm `nvs` is installed. nvs --version ``` -Now we can use nvs to install Node.js v16 and switch on `auto` mode, which will make sure Node.js v16 is used any time we're in the `opentrons` project directory. +Now we can use nvs to install Node.js v18 and switch on `auto` mode, which will make sure Node.js v18 is used any time we're in the `opentrons` project directory. ```shell -nvs add 16 +nvs add 18 nvs auto on ``` @@ -202,7 +202,7 @@ Once you are inside the repository for the first time, you should do two things: 3. Run `python --version` to confirm your chosen version. If you get the incorrect version and you're using an Apple silicon Mac, try running `eval "$(pyenv init --path)"` and then `pyenv local 3.10.13`. Then check `python --version` again. ```shell -# confirm Node v16 +# confirm Node v18 node --version # set Python version, and confirm From 86c138b851a7d5b0329f82633fc2f85c04001ef2 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Mon, 12 Feb 2024 16:00:33 -0500 Subject: [PATCH 15/24] refactor(app): refactor protocol event analytics hooks (#14476) --- .../__tests__/useProtocolRunAnalyticsData.test.tsx | 8 ++++---- .../__tests__/useTrackProtocolRunEvent.test.tsx | 9 +++++++-- .../Devices/hooks/useProtocolRunAnalyticsData.ts | 14 +++++--------- .../hooks/useTrackCreateProtocolRunEvent.ts | 5 ++++- .../Devices/hooks/useTrackProtocolRunEvent.ts | 4 +++- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx index 554a1952c66..709f51f9c0d 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useProtocolRunAnalyticsData.test.tsx @@ -12,6 +12,7 @@ import { useStoredProtocolAnalysis, useProtocolDetailsForRun } from '../' import { useProtocolMetadata } from '../useProtocolMetadata' import { useRunTimestamps } from '../../../RunTimeControl/hooks' import { formatInterval } from '../../../RunTimeControl/utils' +import { mockConnectableRobot } from '../../../../redux/discovery/__fixtures__' jest.mock('../../../../redux/analytics/hash') jest.mock('../../../../redux/protocol-storage') @@ -45,7 +46,6 @@ let store: Store = createStore(jest.fn(), {}) const RUN_ID = '1' const RUN_ID_2 = '2' -const ROBOT_NAME = 'otie' const PIPETTES = [ { id: '1', pipetteName: 'testModelLeft' }, @@ -113,7 +113,7 @@ describe('useProtocolAnalysisErrors hook', () => { it('returns getProtocolRunAnalyticsData function', () => { const { result } = renderHook( - () => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME), + () => useProtocolRunAnalyticsData(RUN_ID, mockConnectableRobot), { wrapper, } @@ -128,7 +128,7 @@ describe('useProtocolAnalysisErrors hook', () => { .calledWith(RUN_ID_2) .mockReturnValue({ protocolData: ROBOT_PROTOCOL_ANALYSIS } as any) const { result } = renderHook( - () => useProtocolRunAnalyticsData(RUN_ID_2, ROBOT_NAME), + () => useProtocolRunAnalyticsData(RUN_ID_2, mockConnectableRobot), { wrapper, } @@ -157,7 +157,7 @@ describe('useProtocolAnalysisErrors hook', () => { it('getProtocolRunAnalyticsData returns fallback stored data when robot data unavailable', async () => { const { result } = renderHook( - () => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME), + () => useProtocolRunAnalyticsData(RUN_ID, mockConnectableRobot), { wrapper, } diff --git a/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx b/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx index 3adff13e65c..5585e923569 100644 --- a/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx +++ b/app/src/organisms/Devices/hooks/__tests__/useTrackProtocolRunEvent.test.tsx @@ -11,6 +11,8 @@ import { useTrackEvent, ANALYTICS_PROTOCOL_RUN_START, } from '../../../../redux/analytics' +import { mockConnectableRobot } from '../../../../redux/discovery/__fixtures__' +import { useRobot } from '../useRobot' jest.mock('../../hooks') jest.mock('../useProtocolRunAnalyticsData') @@ -18,10 +20,12 @@ jest.mock('../../../../redux/discovery') jest.mock('../../../../redux/pipettes') jest.mock('../../../../redux/analytics') jest.mock('../../../../redux/robot-settings') +jest.mock('../useRobot') const mockUseTrackEvent = useTrackEvent as jest.MockedFunction< typeof useTrackEvent > +const mockUseRobot = useRobot as jest.MockedFunction const mockUseProtocolRunAnalyticsData = useProtocolRunAnalyticsData as jest.MockedFunction< typeof useProtocolRunAnalyticsData > @@ -55,8 +59,9 @@ describe('useTrackProtocolRunEvent hook', () => { ) ) mockUseTrackEvent.mockReturnValue(mockTrackEvent) + mockUseRobot.mockReturnValue(mockConnectableRobot) when(mockUseProtocolRunAnalyticsData) - .calledWith(RUN_ID, ROBOT_NAME) + .calledWith(RUN_ID, mockConnectableRobot) .mockReturnValue({ getProtocolRunAnalyticsData: mockGetProtocolRunAnalyticsData, }) @@ -98,7 +103,7 @@ describe('useTrackProtocolRunEvent hook', () => { it('trackProtocolRunEvent calls trackEvent without props when error is thrown in getProtocolRunAnalyticsData', async () => { when(mockUseProtocolRunAnalyticsData) - .calledWith('errorId', ROBOT_NAME) + .calledWith('errorId', mockConnectableRobot) .mockReturnValue({ getProtocolRunAnalyticsData: () => new Promise(() => { diff --git a/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts b/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts index a6b83a93088..93dde4bfefa 100644 --- a/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts +++ b/app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts @@ -3,11 +3,7 @@ import { useSelector } from 'react-redux' import { hash } from '../../../redux/analytics/hash' import { getStoredProtocol } from '../../../redux/protocol-storage' import { getRobotSerialNumber } from '../../../redux/discovery' -import { - useRobot, - useStoredProtocolAnalysis, - useProtocolDetailsForRun, -} from './' +import { useStoredProtocolAnalysis, useProtocolDetailsForRun } from './' import { useProtocolMetadata } from './useProtocolMetadata' import { useRunTimestamps } from '../../RunTimeControl/hooks' import { formatInterval } from '../../RunTimeControl/utils' @@ -17,19 +13,19 @@ import type { ProtocolAnalyticsData } from '../../../redux/analytics/types' import type { StoredProtocolData } from '../../../redux/protocol-storage/types' import type { ProtocolAnalysisOutput } from '@opentrons/shared-data' import type { State } from '../../../redux/types' +import { DiscoveredRobot } from '../../../redux/discovery/types' export const parseProtocolRunAnalyticsData = ( protocolAnalysis: ProtocolAnalysisOutput | null, storedProtocol: StoredProtocolData | null, startedAt: string | null, - robotName: string + robot: DiscoveredRobot | null ) => () => { const hashTasks = [ hash(protocolAnalysis?.metadata?.author) ?? '', hash(storedProtocol?.srcFiles?.toString() ?? '') ?? '', ] - const robot = useRobot(robotName) const serialNumber = robot?.status != null ? getRobotSerialNumber(robot) : null @@ -80,7 +76,7 @@ type GetProtocolRunAnalyticsData = () => Promise<{ */ export function useProtocolRunAnalyticsData( runId: string | null, - robotName: string + robot: DiscoveredRobot | null ): { getProtocolRunAnalyticsData: GetProtocolRunAnalyticsData } { @@ -109,7 +105,7 @@ export function useProtocolRunAnalyticsData( protocolAnalysis as ProtocolAnalysisOutput | null, storedProtocol, startedAt, - robotName + robot ) return { getProtocolRunAnalyticsData } diff --git a/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts b/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts index 400f93fac40..d34da539c8d 100644 --- a/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts +++ b/app/src/organisms/Devices/hooks/useTrackCreateProtocolRunEvent.ts @@ -3,6 +3,7 @@ import { parseProtocolRunAnalyticsData } from './useProtocolRunAnalyticsData' import { parseProtocolAnalysisOutput } from './useStoredProtocolAnalysis' import type { StoredProtocolData } from '../../../redux/protocol-storage' +import { useRobot } from './useRobot' type CreateProtocolRunEventName = | 'createProtocolRecordRequest' @@ -23,6 +24,8 @@ export function useTrackCreateProtocolRunEvent( ): { trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent } { const trackEvent = useTrackEvent() + const robot = useRobot(robotName) + const storedProtocolAnalysis = parseProtocolAnalysisOutput( protocol?.mostRecentAnalysis ?? null ) @@ -31,7 +34,7 @@ export function useTrackCreateProtocolRunEvent( storedProtocolAnalysis, protocol, null, - robotName + robot ) const trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent = ({ diff --git a/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts b/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts index 7cc354805b2..02395c371f2 100644 --- a/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts +++ b/app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts @@ -1,5 +1,6 @@ import { useTrackEvent } from '../../../redux/analytics' import { useProtocolRunAnalyticsData } from './useProtocolRunAnalyticsData' +import { useRobot } from './useRobot' interface ProtocolRunAnalyticsEvent { name: string @@ -15,9 +16,10 @@ export function useTrackProtocolRunEvent( robotName: string ): { trackProtocolRunEvent: TrackProtocolRunEvent } { const trackEvent = useTrackEvent() + const robot = useRobot(robotName) const { getProtocolRunAnalyticsData } = useProtocolRunAnalyticsData( runId, - robotName + robot ) const trackProtocolRunEvent: TrackProtocolRunEvent = ({ From eb025bf877fdec3ba4c4f946e692c45ad48f6fab Mon Sep 17 00:00:00 2001 From: Jeremy Leon Date: Mon, 12 Feb 2024 16:48:51 -0500 Subject: [PATCH 16/24] fix(api): OT-2 trash bins in 2.16 get added to engine at the start of protocol (#14475) --- api/src/opentrons/execute.py | 6 +++-- .../protocol_api/core/engine/protocol.py | 17 +++++++----- .../core/legacy/legacy_protocol_core.py | 6 ++++- .../opentrons/protocol_api/core/protocol.py | 8 +++++- .../protocol_api/create_protocol_context.py | 21 ++++++++++++++- .../protocol_api/protocol_context.py | 26 +++++++++---------- .../protocols/api_support/deck_type.py | 21 ++++++++++++--- api/src/opentrons/simulate.py | 6 +++-- .../protocol_api_integration/test_trashes.py | 10 ------- 9 files changed, 80 insertions(+), 41 deletions(-) diff --git a/api/src/opentrons/execute.py b/api/src/opentrons/execute.py index 6ef5a56e92d..8713161eb67 100644 --- a/api/src/opentrons/execute.py +++ b/api/src/opentrons/execute.py @@ -41,7 +41,7 @@ from opentrons.protocols.api_support.deck_type import ( guess_from_global_config as guess_deck_type_from_global_config, should_load_fixed_trash, - should_load_fixed_trash_for_python_protocol, + should_load_fixed_trash_labware_for_python_protocol, ) from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.execution import execute as execute_apiv2 @@ -540,7 +540,9 @@ def _create_live_context_pe( config=_get_protocol_engine_config(), drop_tips_after_run=False, post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, - load_fixed_trash=should_load_fixed_trash_for_python_protocol(api_version), + load_fixed_trash=should_load_fixed_trash_labware_for_python_protocol( + api_version + ), ) ) diff --git a/api/src/opentrons/protocol_api/core/engine/protocol.py b/api/src/opentrons/protocol_api/core/engine/protocol.py index 501dccc3621..17c9c9bcec9 100644 --- a/api/src/opentrons/protocol_api/core/engine/protocol.py +++ b/api/src/opentrons/protocol_api/core/engine/protocol.py @@ -134,9 +134,14 @@ def _load_fixed_trash(self) -> None: def append_disposal_location( self, disposal_location: Union[Labware, TrashBin, WasteChute], - skip_add_to_engine: bool = False, ) -> None: - """Append a disposal location object to the core""" + """Append a disposal location object to the core.""" + self._disposal_locations.append(disposal_location) + + def add_disposal_location_to_engine( + self, disposal_location: Union[TrashBin, WasteChute] + ) -> None: + """Verify and add disposal location to engine store and append it to the core.""" if isinstance(disposal_location, TrashBin): self._engine_client.state.addressable_areas.raise_if_area_not_in_deck_configuration( disposal_location.area_name @@ -152,8 +157,7 @@ def append_disposal_location( existing_labware_ids=list(self._labware_cores_by_id.keys()), existing_module_ids=list(self._module_cores_by_id.keys()), ) - if not skip_add_to_engine: - self._engine_client.add_addressable_area(disposal_location.area_name) + self._engine_client.add_addressable_area(disposal_location.area_name) elif isinstance(disposal_location, WasteChute): # TODO(jbl 2024-01-25) hardcoding this specific addressable area should be refactored # when analysis is fixed up @@ -166,9 +170,8 @@ def append_disposal_location( self._engine_client.state.addressable_areas.raise_if_area_not_in_deck_configuration( "1ChannelWasteChute" ) - if not skip_add_to_engine: - self._engine_client.add_addressable_area("1ChannelWasteChute") - self._disposal_locations.append(disposal_location) + self._engine_client.add_addressable_area("1ChannelWasteChute") + self.append_disposal_location(disposal_location) def get_disposal_locations(self) -> List[Union[Labware, TrashBin, WasteChute]]: """Get disposal locations.""" diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py index 2c2ac1eefe0..36518f68494 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py @@ -136,7 +136,6 @@ def is_simulating(self) -> bool: def append_disposal_location( self, disposal_location: Union[Labware, TrashBin, WasteChute], - skip_add_to_engine: bool = False, ) -> None: if isinstance(disposal_location, (TrashBin, WasteChute)): raise APIVersionError( @@ -144,6 +143,11 @@ def append_disposal_location( ) self._disposal_locations.append(disposal_location) + def add_disposal_location_to_engine( + self, disposal_location: Union[TrashBin, WasteChute] + ) -> None: + assert False, "add_disposal_location_to_engine only supported on engine core" + def add_labware_definition( self, definition: LabwareDefinition, diff --git a/api/src/opentrons/protocol_api/core/protocol.py b/api/src/opentrons/protocol_api/core/protocol.py index 8443408eb1b..7c198646905 100644 --- a/api/src/opentrons/protocol_api/core/protocol.py +++ b/api/src/opentrons/protocol_api/core/protocol.py @@ -65,11 +65,17 @@ def add_labware_definition( def append_disposal_location( self, disposal_location: Union[Labware, TrashBin, WasteChute], - skip_add_to_engine: bool = False, ) -> None: """Append a disposal location object to the core""" ... + @abstractmethod + def add_disposal_location_to_engine( + self, disposal_location: Union[TrashBin, WasteChute] + ) -> None: + """Verify and add disposal location to engine store and append it to the core.""" + ... + @abstractmethod def load_labware( self, diff --git a/api/src/opentrons/protocol_api/create_protocol_context.py b/api/src/opentrons/protocol_api/create_protocol_context.py index 5a64e70cf99..22832911c90 100644 --- a/api/src/opentrons/protocol_api/create_protocol_context.py +++ b/api/src/opentrons/protocol_api/create_protocol_context.py @@ -15,10 +15,14 @@ from opentrons.protocol_engine import ProtocolEngine from opentrons.protocol_engine.clients import SyncClient, ChildThreadTransport from opentrons.protocols.api_support.types import APIVersion +from opentrons.protocols.api_support.deck_type import ( + should_load_fixed_trash_area_for_python_protocol, +) from opentrons.protocols.api_support.definitions import MAX_SUPPORTED_VERSION from .protocol_context import ProtocolContext from .deck import Deck +from ._trash_bin import TrashBin from .core.common import ProtocolCore as AbstractProtocolCore from .core.legacy.deck import Deck as LegacyDeck @@ -148,7 +152,7 @@ def create_protocol_context( # this swap may happen once `ctx.move_labware` off-deck is implemented deck = None if isinstance(core, ProtocolCore) else cast(Deck, core.get_deck()) - return ProtocolContext( + context = ProtocolContext( api_version=api_version, # TODO(mm, 2023-05-11): This cast shouldn't be necessary. # Fix this by making the appropriate TypeVars covariant? @@ -158,3 +162,18 @@ def create_protocol_context( deck=deck, bundled_data=bundled_data, ) + # If we're loading an engine based core into the context, and we're on api level 2.16 or above, on an OT-2 we need + # to insert a fixed trash addressable area into the protocol engine, for correctness in anything that relies on + # knowing what addressable areas have been loaded (and any checks involving trash geometry). Because the method + # that uses this in the core relies on the sync client and this code will run in the main thread (which if called + # will cause a deadlock), we're directly calling the protocol engine method here where we have access to it. + if ( + protocol_engine is not None + and should_load_fixed_trash_area_for_python_protocol( + api_version=api_version, + robot_type=protocol_engine.state_view.config.robot_type, + ) + ): + assert isinstance(context.fixed_trash, TrashBin) + protocol_engine.add_addressable_area(context.fixed_trash.area_name) + return context diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index 3920f6446f9..e7c6e63de8d 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -26,7 +26,8 @@ from opentrons.protocols.api_support import instrument as instrument_support from opentrons.protocols.api_support.deck_type import ( NoTrashDefinedError, - should_load_fixed_trash_for_python_protocol, + should_load_fixed_trash_labware_for_python_protocol, + should_load_fixed_trash_area_for_python_protocol, ) from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.api_support.util import ( @@ -159,22 +160,19 @@ def __init__( # protocols after 2.16 expect trash to exist as either a TrashBin or WasteChute object. self._load_fixed_trash() - if should_load_fixed_trash_for_python_protocol(self._api_version): + if should_load_fixed_trash_labware_for_python_protocol(self._api_version): self._core.append_disposal_location(self.fixed_trash) - elif ( - self._api_version >= APIVersion(2, 16) - and self._core.robot_type == "OT-2 Standard" + elif should_load_fixed_trash_area_for_python_protocol( + self._api_version, self._core.robot_type ): _fixed_trash_trashbin = TrashBin( location=DeckSlotName.FIXED_TRASH, addressable_area_name="fixedTrash" ) - # We have to skip adding this fixed trash bin to engine because this __init__ is called in the main thread - # and any calls to sync client will cause a deadlock. This means that OT-2 fixed trashes are not added to - # the engine store until one is first referenced. This should have minimal consequences for OT-2 given that - # we do not need to worry about the 96 channel pipette and partial tip configuration with that pipette. - self._core.append_disposal_location( - _fixed_trash_trashbin, skip_add_to_engine=True - ) + # We are just appending the fixed trash to the core's internal list here, not adding it to the engine via + # the core, since that method works through the SyncClient and if called from here, will cause protocols + # to deadlock. Instead, that method is called in protocol engine directly in create_protocol_context after + # ProtocolContext is initialized. + self._core.append_disposal_location(_fixed_trash_trashbin) self._commands: List[str] = [] self._unsubscribe_commands: Optional[Callable[[], None]] = None @@ -517,7 +515,7 @@ def load_trash_bin(self, location: DeckLocation) -> TrashBin: trash_bin = TrashBin( location=slot_name, addressable_area_name=addressable_area_name ) - self._core.append_disposal_location(trash_bin) + self._core.add_disposal_location_to_engine(trash_bin) return trash_bin @requires_version(2, 16) @@ -534,7 +532,7 @@ def load_waste_chute( API will raise an error. """ waste_chute = WasteChute() - self._core.append_disposal_location(waste_chute) + self._core.add_disposal_location_to_engine(waste_chute) return waste_chute @requires_version(2, 15) diff --git a/api/src/opentrons/protocols/api_support/deck_type.py b/api/src/opentrons/protocols/api_support/deck_type.py index f0cadebce43..4bd70c5fc28 100644 --- a/api/src/opentrons/protocols/api_support/deck_type.py +++ b/api/src/opentrons/protocols/api_support/deck_type.py @@ -45,15 +45,30 @@ def __init__( ) -def should_load_fixed_trash_for_python_protocol(api_version: APIVersion) -> bool: +def should_load_fixed_trash_labware_for_python_protocol( + api_version: APIVersion, +) -> bool: + """Whether to automatically load the fixed trash as a labware for a Python protocol at protocol start.""" return api_version <= LOAD_FIXED_TRASH_GATE_VERSION_PYTHON +def should_load_fixed_trash_area_for_python_protocol( + api_version: APIVersion, robot_type: RobotType +) -> bool: + """Whether to automatically load the fixed trash addressable area for OT-2 protocols on 2.16 and above.""" + return ( + api_version > LOAD_FIXED_TRASH_GATE_VERSION_PYTHON + and robot_type == "OT-2 Standard" + ) + + def should_load_fixed_trash(protocol_config: ProtocolConfig) -> bool: - """Decide whether to automatically load fixed trash on the deck based on version.""" + """Decide whether to automatically load fixed trash labware on the deck based on version.""" load_fixed_trash = False if isinstance(protocol_config, PythonProtocolConfig): - return should_load_fixed_trash_for_python_protocol(protocol_config.api_version) + return should_load_fixed_trash_labware_for_python_protocol( + protocol_config.api_version + ) # TODO(jbl 2023-10-27), when schema v8 is out, use a new deck version field to support fixed trash protocols elif isinstance(protocol_config, JsonProtocolConfig): load_fixed_trash = ( diff --git a/api/src/opentrons/simulate.py b/api/src/opentrons/simulate.py index 5e8433debba..16d6859530f 100644 --- a/api/src/opentrons/simulate.py +++ b/api/src/opentrons/simulate.py @@ -66,7 +66,7 @@ from opentrons.protocols.api_support.deck_type import ( for_simulation as deck_type_for_simulation, should_load_fixed_trash, - should_load_fixed_trash_for_python_protocol, + should_load_fixed_trash_labware_for_python_protocol, ) from opentrons.protocols.api_support.types import APIVersion from opentrons_shared_data.labware.labware_definition import LabwareDefinition @@ -801,7 +801,9 @@ def _create_live_context_pe( config=_get_protocol_engine_config(robot_type), drop_tips_after_run=False, post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, - load_fixed_trash=should_load_fixed_trash_for_python_protocol(api_version), + load_fixed_trash=should_load_fixed_trash_labware_for_python_protocol( + api_version + ), ) ) diff --git a/api/tests/opentrons/protocol_api_integration/test_trashes.py b/api/tests/opentrons/protocol_api_integration/test_trashes.py index f687e497c93..1c8250fe44e 100644 --- a/api/tests/opentrons/protocol_api_integration/test_trashes.py +++ b/api/tests/opentrons/protocol_api_integration/test_trashes.py @@ -123,16 +123,6 @@ def test_trash_search() -> None: "2.16", "OT-2", False, - # This should ideally raise, matching OT-2 behavior on prior Protocol API versions. - # It currently does not because Protocol API v2.15's trashes are implemented as - # addressable areas, not labware--and they're only brought into existence - # *on first use,* not at the beginning of a protocol. - # - # The good news is that even though the conflicting load will not raise like we want, - # something in the protocol will eventually raise, e.g. when a pipette goes to drop a - # tip in the fixed trash and finds that a fixed trash can't exist there because there's - # a labware. - marks=pytest.mark.xfail(strict=True, raises=pytest.fail.Exception), ), pytest.param( "2.16", From b44a379dd5bcb382571c81d95e7e2cd9c134d153 Mon Sep 17 00:00:00 2001 From: Jethary Rader <66035149+jerader@users.noreply.github.com> Date: Mon, 12 Feb 2024 17:07:11 -0500 Subject: [PATCH 17/24] fix(app): remove [Object, object] tooltip from appearing on hover (#14474) closes RQA-2323 --- app/src/molecules/LegacyModal/index.tsx | 2 +- app/src/organisms/AnalyticsSettingsModal/index.tsx | 2 +- .../UpdateAppModal/__tests__/UpdateAppModal.test.tsx | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/molecules/LegacyModal/index.tsx b/app/src/molecules/LegacyModal/index.tsx index 189abd4b700..b6e9b514093 100644 --- a/app/src/molecules/LegacyModal/index.tsx +++ b/app/src/molecules/LegacyModal/index.tsx @@ -68,7 +68,7 @@ export const LegacyModal = (props: LegacyModalProps): JSX.Element => { onOutsideClick={closeOnOutsideClick ?? false ? onClose : undefined} // center within viewport aside from nav marginLeft={styleProps.marginLeft ?? '7.125rem'} - {...props} + {...styleProps} footer={footer} > {children} diff --git a/app/src/organisms/AnalyticsSettingsModal/index.tsx b/app/src/organisms/AnalyticsSettingsModal/index.tsx index bba698037aa..602c5086380 100644 --- a/app/src/organisms/AnalyticsSettingsModal/index.tsx +++ b/app/src/organisms/AnalyticsSettingsModal/index.tsx @@ -58,7 +58,7 @@ export function AnalyticsSettingsModal(): JSX.Element | null { }} /> diff --git a/app/src/organisms/UpdateAppModal/__tests__/UpdateAppModal.test.tsx b/app/src/organisms/UpdateAppModal/__tests__/UpdateAppModal.test.tsx index 2e38c579982..f3473854489 100644 --- a/app/src/organisms/UpdateAppModal/__tests__/UpdateAppModal.test.tsx +++ b/app/src/organisms/UpdateAppModal/__tests__/UpdateAppModal.test.tsx @@ -129,6 +129,8 @@ describe('UpdateAppModal', () => { error: { name: 'Update Error' }, } as ShellUpdateState) render(props) - expect(screen.getByTitle('Update Error')).toBeInTheDocument() + expect( + screen.getByRole('heading', { name: 'Update Error' }) + ).toBeInTheDocument() }) }) From 1fb881f168aff9188c924b5b991066746d1b38cb Mon Sep 17 00:00:00 2001 From: Alise Au <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 12 Feb 2024 17:34:26 -0500 Subject: [PATCH 18/24] fix(api): do not include HEPA-UV in motor nodes (#14478) --- api/src/opentrons/hardware_control/backends/ot3utils.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/src/opentrons/hardware_control/backends/ot3utils.py b/api/src/opentrons/hardware_control/backends/ot3utils.py index 3cc4b75bc76..d585a48f99d 100644 --- a/api/src/opentrons/hardware_control/backends/ot3utils.py +++ b/api/src/opentrons/hardware_control/backends/ot3utils.py @@ -351,8 +351,13 @@ def motor_nodes(devices: Set[FirmwareTarget]) -> Set[NodeId]: NodeId.head_bootloader, NodeId.gripper_bootloader, } + hepa_uv_nodes = { + NodeId.hepa_uv, + NodeId.hepa_uv_bootloader, + } # remove any bootloader nodes motor_nodes -= bootloader_nodes + motor_nodes -= hepa_uv_nodes # filter out usb nodes return {NodeId(target) for target in motor_nodes if target in NodeId} From 15986f074d36e5d008d9631f048cee5396e8819b Mon Sep 17 00:00:00 2001 From: Brent Hagen Date: Mon, 12 Feb 2024 18:20:24 -0500 Subject: [PATCH 19/24] feat(components): adjust touchscreen greens and purples (#14481) toggles between slightly different greens and purples based on window.matchMedia matching to the touchscreen height and width closes RAUT-966 --- components/src/helix-design-system/colors.ts | 24 +++++++++++-------- .../src/ui-style-constants/responsiveness.ts | 5 ++++ scripts/setup-global-mocks.js | 16 +++++++++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/components/src/helix-design-system/colors.ts b/components/src/helix-design-system/colors.ts index ee67be9d588..348ab87626e 100644 --- a/components/src/helix-design-system/colors.ts +++ b/components/src/helix-design-system/colors.ts @@ -1,10 +1,14 @@ +// some colors are slightly adjusted for on-device display screen differences +// isTouchscreen hex values are dev concerns only - designs will reflect the standard hex values +import { isTouchscreen } from '../ui-style-constants/responsiveness' + /** * green */ export const green60 = '#03683E' -export const green50 = '#04AA65' -export const green40 = '#91E2C0' -export const green35 = '#AFEDD3' +export const green50 = isTouchscreen ? '#1CA850' : '#04AA65' +export const green40 = isTouchscreen ? '#8EF3A8' : '#91E2C0' +export const green35 = isTouchscreen ? '#8AFBAB' : '#AFEDD3' export const green30 = '#C4F6E0' export const green20 = '#E8F7ED' @@ -32,13 +36,13 @@ export const yellow20 = '#FDF3E2' /** * purple */ -export const purple60 = '#562566' -export const purple55 = '#713187' -export const purple50 = '#893BA4' -export const purple40 = '#CEA4DF' -export const purple35 = '#DBBCE7' -export const purple30 = '#E6D5EC' -export const purple20 = '#F1E8F5' +export const purple60 = isTouchscreen ? '#612367' : '#562566' +export const purple55 = isTouchscreen ? '#822E89' : '#713187' +export const purple50 = isTouchscreen ? '#9E39A8' : '#893BA4' +export const purple40 = isTouchscreen ? '#E2A9EA' : '#CEA4DF' +export const purple35 = isTouchscreen ? '#ECC2F2' : '#DBBCE7' +export const purple30 = isTouchscreen ? '#F4DEF7' : '#E6D5EC' +export const purple20 = isTouchscreen ? '#FFF3FE' : '#F1E8F5' /** * blue diff --git a/components/src/ui-style-constants/responsiveness.ts b/components/src/ui-style-constants/responsiveness.ts index 8925e2eabab..d1a3e2ec8ef 100644 --- a/components/src/ui-style-constants/responsiveness.ts +++ b/components/src/ui-style-constants/responsiveness.ts @@ -4,3 +4,8 @@ // before the release of this code to prevent Funny desktop app behavior when the viewport // is precisely 600x1024 export const touchscreenMediaQuerySpecs = '(height: 600px) and (width: 1024px)' + +export const isTouchscreen = + typeof window === 'object' && window.matchMedia != null + ? window.matchMedia(touchscreenMediaQuerySpecs).matches + : false diff --git a/scripts/setup-global-mocks.js b/scripts/setup-global-mocks.js index 79333ac496c..95d505371b9 100644 --- a/scripts/setup-global-mocks.js +++ b/scripts/setup-global-mocks.js @@ -27,3 +27,19 @@ jest.mock('../protocol-designer/src/components/portals/MainPageModalPortal') jest.mock('typeface-open-sans', () => {}) jest.mock('@fontsource/dejavu-sans', () => {}) jest.mock('@fontsource/public-sans', () => {}) + +// jest requires methods not implemented by JSDOM to be mocked, e.g. window.matchMedia +// https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation(query => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // deprecated + removeListener: jest.fn(), // deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +}) From ebe71640db12edf09d65cf7aa5c32ac6781892dc Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Tue, 13 Feb 2024 16:26:19 -0500 Subject: [PATCH 20/24] fix(app): fix RecentRunProtocolCard redirecting to /protocols before /runs/:runId/setup (#14486) Closes RQA-2290 Notification changes uncovered a bug in which a RecentRunProtocolCard caused redirection to the /runs/:runId/setup page before the actual run had loaded, which caused the TopLevelRedirect hook to bounce back to the /protocols page until the run loads. Instead of relying on run within TopLevelRedirect for this one off route case, move the redirect within ProtocolSetup and redirect if run status is stopped. This creates the same end behavior without the temporary redirect to /protocols. --- app/src/App/hooks.ts | 4 ---- .../__tests__/ProtocolSetup.test.tsx | 17 ++++++++++++++++- app/src/pages/ProtocolSetup/index.tsx | 8 ++++++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/src/App/hooks.ts b/app/src/App/hooks.ts index 4c0faf2c7e8..cbc40b396eb 100644 --- a/app/src/App/hooks.ts +++ b/app/src/App/hooks.ts @@ -2,7 +2,6 @@ import * as React from 'react' import difference from 'lodash/difference' import { useTranslation } from 'react-i18next' import { useQueryClient } from 'react-query' -import { useRouteMatch } from 'react-router-dom' import { useDispatch } from 'react-redux' import { useInterval, truncateString } from '@opentrons/components' @@ -146,9 +145,6 @@ export function useCurrentRunRoute(): string | null { enabled: currentRunId != null, }) - const isRunSetupRoute = useRouteMatch('/runs/:runId/setup') - if (isRunSetupRoute != null && runRecord == null) return '/protocols' - const runStatus = runRecord?.data.status const runActions = runRecord?.data.actions if (runRecord == null || runStatus == null || runActions == null) return null diff --git a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx index ca6d6b275f0..d08d1d010d0 100644 --- a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx +++ b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx @@ -4,7 +4,7 @@ import { MemoryRouter } from 'react-router-dom' import { fireEvent, screen } from '@testing-library/react' import { when, resetAllWhenMocks } from 'jest-when' -import { RUN_STATUS_IDLE } from '@opentrons/api-client' +import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED } from '@opentrons/api-client' import { useAllPipetteOffsetCalibrationsQuery, useInstrumentsQuery, @@ -72,6 +72,8 @@ Object.defineProperty(window, 'IntersectionObserver', { value: IntersectionObserver, }) +let mockHistoryPush: jest.Mock + jest.mock('@opentrons/shared-data/js/helpers') jest.mock('@opentrons/react-api-client') jest.mock('../../../organisms/LabwarePositionCheck/useLaunchLPC') @@ -91,6 +93,12 @@ jest.mock('../ConfirmAttachedModal') jest.mock('../../../organisms/ToasterOven') jest.mock('../../../resources/deck_configuration/hooks') jest.mock('../../../resources/runs/useNotifyRunQuery') +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useHistory: () => ({ + push: mockHistoryPush, + }), +})) const mockGetDeckDefFromRobotType = getDeckDefFromRobotType as jest.MockedFunction< typeof getDeckDefFromRobotType @@ -252,6 +260,7 @@ describe('ProtocolSetup', () => { let mockLaunchLPC: jest.Mock beforeEach(() => { mockLaunchLPC = jest.fn() + mockHistoryPush = jest.fn() mockUseLPCDisabledReason.mockReturnValue(null) mockUseAttachedModules.mockReturnValue([]) mockProtocolSetupModulesAndDeck.mockReturnValue( @@ -462,4 +471,10 @@ describe('ProtocolSetup', () => { properties: {}, }) }) + + it('should redirect to the protocols page when a run is stopped', () => { + mockUseRunStatus.mockReturnValue(RUN_STATUS_STOPPED) + render(`/runs/${RUN_ID}/setup/`) + expect(mockHistoryPush).toHaveBeenCalledWith('/protocols') + }) }) diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 950eb60991e..e305920746f 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -269,6 +269,11 @@ function PrepareToRun({ } ) + const runStatus = useRunStatus(runId) + if (runStatus === RUN_STATUS_STOPPED) { + history.push('/protocols') + } + React.useEffect(() => { if (mostRecentAnalysis?.status === 'completed') { setIsPollingForCompletedAnalysis(false) @@ -305,7 +310,6 @@ function PrepareToRun({ const protocolHasFixtures = requiredFixtures.length > 0 - const runStatus = useRunStatus(runId) const isHeaterShakerInProtocol = useIsHeaterShakerInProtocol() const deckDef = getDeckDefFromRobotType(robotType) @@ -450,7 +454,7 @@ function PrepareToRun({ if ( isHeaterShakerInProtocol && isReadyToRun && - (runStatus === RUN_STATUS_IDLE || runStatus === RUN_STATUS_STOPPED) + runStatus === RUN_STATUS_IDLE ) { confirmAttachment() } else { From 9badd57994ed82b683fc50eccd285c2c75af43c7 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Tue, 13 Feb 2024 16:41:20 -0500 Subject: [PATCH 21/24] fix(app-shell, app-shell-odd): Fix intermittently dropped notifications (#14477) Closes RQA-2339, RQA-2321, RQA-2319, RAUT-962, RQA-2346 * fix(app-shell, app-shell-odd): fix intermittently dropped notifications Because subscription logic is directly tied to the component lifecycle, it is possible for a component to trigger an unsubscribe event on dismount while a new component mounts and triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions, do not unsubscribe and close connections before newly mounted components have had time to update the connection store. --- app-shell-odd/src/notify.ts | 48 +++++++++++++++++++++---------------- app-shell/src/notify.ts | 48 +++++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 40 deletions(-) diff --git a/app-shell-odd/src/notify.ts b/app-shell-odd/src/notify.ts index 69aea75e39b..be0ff21310d 100644 --- a/app-shell-odd/src/notify.ts +++ b/app-shell-odd/src/notify.ts @@ -184,30 +184,38 @@ function subscribe(notifyParams: NotifyParams): Promise { } } +// Because subscription logic is directly tied to the component lifecycle, it is possible +// for a component to trigger an unsubscribe event on dismount while a new component mounts and +// triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions, +// do not unsubscribe and close connections before newly mounted components have had time to update the connection store. +const RENDER_TIMEOUT = 10000 // 10 seconds + function unsubscribe(notifyParams: NotifyParams): Promise { const { hostname, topic } = notifyParams return new Promise(() => { if (hostname in connectionStore) { - const { client } = connectionStore[hostname] - const subscriptions = connectionStore[hostname]?.subscriptions - const isLastSubscription = subscriptions[topic] <= 1 - - if (isLastSubscription) { - client?.unsubscribe(topic, {}, (error, result) => { - if (error != null) { - log.warn( - `Failed to unsubscribe on ${hostname} from topic: ${topic}` - ) - } else { - log.info( - `Successfully unsubscribed on ${hostname} from topic: ${topic}` - ) - handleDecrementSubscriptionCount(hostname, topic) - } - }) - } else { - subscriptions[topic] -= 1 - } + setTimeout(() => { + const { client } = connectionStore[hostname] + const subscriptions = connectionStore[hostname]?.subscriptions + const isLastSubscription = subscriptions[topic] <= 1 + + if (isLastSubscription) { + client?.unsubscribe(topic, {}, (error, result) => { + if (error != null) { + log.warn( + `Failed to unsubscribe on ${hostname} from topic: ${topic}` + ) + } else { + log.info( + `Successfully unsubscribed on ${hostname} from topic: ${topic}` + ) + handleDecrementSubscriptionCount(hostname, topic) + } + }) + } else { + subscriptions[topic] -= 1 + } + }, RENDER_TIMEOUT) } else { log.info( `Attempted to unsubscribe from unconnected hostname: ${hostname}` diff --git a/app-shell/src/notify.ts b/app-shell/src/notify.ts index 822dcebd084..da1a580b81e 100644 --- a/app-shell/src/notify.ts +++ b/app-shell/src/notify.ts @@ -180,30 +180,38 @@ function subscribe(notifyParams: NotifyParams): Promise { } } +// Because subscription logic is directly tied to the component lifecycle, it is possible +// for a component to trigger an unsubscribe event on dismount while a new component mounts and +// triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions, +// do not unsubscribe and close connections before newly mounted components have had time to update the connection store. +const RENDER_TIMEOUT = 10000 // 10 seconds + function unsubscribe(notifyParams: NotifyParams): Promise { const { hostname, topic } = notifyParams return new Promise(() => { if (hostname in connectionStore) { - const { client } = connectionStore[hostname] - const subscriptions = connectionStore[hostname]?.subscriptions - const isLastSubscription = subscriptions[topic] <= 1 - - if (isLastSubscription) { - client?.unsubscribe(topic, {}, (error, result) => { - if (error != null) { - log.warn( - `Failed to unsubscribe on ${hostname} from topic: ${topic}` - ) - } else { - log.info( - `Successfully unsubscribed on ${hostname} from topic: ${topic}` - ) - handleDecrementSubscriptionCount(hostname, topic) - } - }) - } else { - subscriptions[topic] -= 1 - } + setTimeout(() => { + const { client } = connectionStore[hostname] + const subscriptions = connectionStore[hostname]?.subscriptions + const isLastSubscription = subscriptions[topic] <= 1 + + if (isLastSubscription) { + client?.unsubscribe(topic, {}, (error, result) => { + if (error != null) { + log.warn( + `Failed to unsubscribe on ${hostname} from topic: ${topic}` + ) + } else { + log.info( + `Successfully unsubscribed on ${hostname} from topic: ${topic}` + ) + handleDecrementSubscriptionCount(hostname, topic) + } + }) + } else { + subscriptions[topic] -= 1 + } + }, RENDER_TIMEOUT) } else { log.info( `Attempted to unsubscribe from unconnected hostname: ${hostname}` From 23146bae5f8ed0bc1deebf0c488106308150b955 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Tue, 13 Feb 2024 17:08:14 -0500 Subject: [PATCH 22/24] fix(app): fix LPC offsets not appropriately updating (#14487) Closes RQA-2296 The staleTime property was not handled correctly by the useNotifyService notification hook. In addition to disabling notifications when using staleTime, we must also alert the HTTP hook that it is responsible for refetching HTTP data. --- .../maintenance_runs/useNotifyCurrentMaintenanceRun.ts | 6 +++++- app/src/resources/runs/useNotifyAllRunsQuery.ts | 6 +++++- app/src/resources/runs/useNotifyRunQuery.ts | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts index 9cc84e6c3d5..4c634225958 100644 --- a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts +++ b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts @@ -23,7 +23,11 @@ export function useNotifyCurrentMaintenanceRun( }, }) - const isNotifyEnabled = !isNotifyError && !options?.forceHttpPolling + const isNotifyEnabled = + !isNotifyError && + !options?.forceHttpPolling && + options?.staleTime !== Infinity + if (!isNotifyEnabled && !refetchUsingHTTP) setRefetchUsingHTTP(true) const isHTTPEnabled = host !== null && options?.enabled !== false && refetchUsingHTTP diff --git a/app/src/resources/runs/useNotifyAllRunsQuery.ts b/app/src/resources/runs/useNotifyAllRunsQuery.ts index f8631582495..e8d1d8c0478 100644 --- a/app/src/resources/runs/useNotifyAllRunsQuery.ts +++ b/app/src/resources/runs/useNotifyAllRunsQuery.ts @@ -26,7 +26,11 @@ export function useNotifyAllRunsQuery( options, }) - const isNotifyEnabled = !isNotifyError && !options?.forceHttpPolling + const isNotifyEnabled = + !isNotifyError && + !options?.forceHttpPolling && + options?.staleTime !== Infinity + if (!isNotifyEnabled && !refetchUsingHTTP) setRefetchUsingHTTP(true) const isHTTPEnabled = options?.enabled !== false && refetchUsingHTTP diff --git a/app/src/resources/runs/useNotifyRunQuery.ts b/app/src/resources/runs/useNotifyRunQuery.ts index d70298c2377..fb7b5442bbd 100644 --- a/app/src/resources/runs/useNotifyRunQuery.ts +++ b/app/src/resources/runs/useNotifyRunQuery.ts @@ -22,7 +22,11 @@ export function useNotifyRunQuery( options: { ...options, enabled: options.enabled && runId != null }, }) - const isNotifyEnabled = !isNotifyError && !options?.forceHttpPolling + const isNotifyEnabled = + !isNotifyError && + !options?.forceHttpPolling && + options?.staleTime !== Infinity + if (!isNotifyEnabled && !refetchUsingHTTP) setRefetchUsingHTTP(true) const isHTTPEnabled = options?.enabled !== false && From a788fa4b9c57a5d57233af91c985cf81526adb4d Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 14 Feb 2024 12:20:56 -0500 Subject: [PATCH 23/24] perf(robot-server): Parallelize the migration to schema 3 (#14465) --- .../robot_server/persistence/__init__.py | 7 +- .../robot_server/persistence/_database.py | 43 ++-- .../persistence/_fastapi_dependencies.py | 4 +- .../_migrations/_up_to_3_worker.py | 111 +++++++++++ .../persistence/_migrations/up_to_3.py | 188 +++++++++++------- .../persistence/_tables/schema_2.py | 2 +- .../robot_server/persistence/legacy_pickle.py | 4 +- robot-server/tests/conftest.py | 8 +- 8 files changed, 253 insertions(+), 114 deletions(-) create mode 100644 robot-server/robot_server/persistence/_migrations/_up_to_3_worker.py diff --git a/robot-server/robot_server/persistence/__init__.py b/robot-server/robot_server/persistence/__init__.py index 604c331f1c5..ad521aca62f 100644 --- a/robot-server/robot_server/persistence/__init__.py +++ b/robot-server/robot_server/persistence/__init__.py @@ -1,7 +1,7 @@ """Support for persisting data across device reboots.""" -from ._database import create_schema_3_sql_engine, sqlite_rowid +from ._database import create_sql_engine, sql_engine_ctx, sqlite_rowid from ._fastapi_dependencies import ( start_initializing_persistence, clean_up_persistence, @@ -12,6 +12,7 @@ ) from ._persistence_directory import PersistenceResetter from ._tables import ( + metadata, protocol_table, analysis_table, run_table, @@ -22,9 +23,11 @@ __all__ = [ # database utilities and helpers - "create_schema_3_sql_engine", + "create_sql_engine", + "sql_engine_ctx", "sqlite_rowid", # database tables + "metadata", "protocol_table", "analysis_table", "run_table", diff --git a/robot-server/robot_server/persistence/_database.py b/robot-server/robot_server/persistence/_database.py index 3110994bb5b..7204e47517f 100644 --- a/robot-server/robot_server/persistence/_database.py +++ b/robot-server/robot_server/persistence/_database.py @@ -1,13 +1,12 @@ """SQLite database initialization and utilities.""" +from contextlib import contextmanager from pathlib import Path +from typing import Generator import sqlalchemy from server_utils import sql_utils -from ._tables import schema_2, schema_3 -from ._migrations.up_to_2 import migrate - # A reference to SQLite's built-in ROWID column. # @@ -24,23 +23,17 @@ sqlite_rowid = sqlalchemy.column("_ROWID_") -def create_schema_2_sql_engine(path: Path) -> sqlalchemy.engine.Engine: - """Create a SQL engine for a schema 2 database. - - If provided a schema 0 or 1 database, this will migrate it in-place to schema 2. +def create_sql_engine(path: Path) -> sqlalchemy.engine.Engine: + """Return an engine for accessing the given SQLite database file. - Warning: - Migrations can take several minutes. If calling this from an async function, - offload this to a thread to avoid blocking the event loop. + If the file does not already exist, it will be created, empty. + You must separately set up any tables you're expecting. """ sql_engine = sqlalchemy.create_engine(sql_utils.get_connection_url(path)) try: sql_utils.enable_foreign_key_constraints(sql_engine) sql_utils.fix_transactions(sql_engine) - schema_2.metadata.create_all(sql_engine) - - migrate(sql_engine) except Exception: sql_engine.dispose() @@ -49,21 +42,11 @@ def create_schema_2_sql_engine(path: Path) -> sqlalchemy.engine.Engine: return sql_engine -def create_schema_3_sql_engine(path: Path) -> sqlalchemy.engine.Engine: - """Create a SQL engine for a schema 3 database. - - Unlike `create_schema_2_sql_engine()`, this assumes the database is already - at schema 3. Migration is done through other mechanisms. - """ - sql_engine = sqlalchemy.create_engine(sql_utils.get_connection_url(path)) - +@contextmanager +def sql_engine_ctx(path: Path) -> Generator[sqlalchemy.engine.Engine, None, None]: + """Like `create_sql_engine()`, but clean up when done.""" + engine = create_sql_engine(path) try: - sql_utils.enable_foreign_key_constraints(sql_engine) - sql_utils.fix_transactions(sql_engine) - schema_3.metadata.create_all(sql_engine) - - except Exception: - sql_engine.dispose() - raise - - return sql_engine + yield engine + finally: + engine.dispose() diff --git a/robot-server/robot_server/persistence/_fastapi_dependencies.py b/robot-server/robot_server/persistence/_fastapi_dependencies.py index 7a2e32a1575..ebdafb70e87 100644 --- a/robot-server/robot_server/persistence/_fastapi_dependencies.py +++ b/robot-server/robot_server/persistence/_fastapi_dependencies.py @@ -16,7 +16,7 @@ ) from robot_server.errors import ErrorDetails -from ._database import create_schema_3_sql_engine +from ._database import create_sql_engine from ._persistence_directory import ( PersistenceResetter, prepare_active_subdirectory, @@ -102,7 +102,7 @@ async def init_sql_engine() -> SQLEngine: prepared_subdirectory = await subdirectory_prep_task sql_engine = await to_thread.run_sync( - create_schema_3_sql_engine, prepared_subdirectory / _DATABASE_FILE + create_sql_engine, prepared_subdirectory / _DATABASE_FILE ) return sql_engine diff --git a/robot-server/robot_server/persistence/_migrations/_up_to_3_worker.py b/robot-server/robot_server/persistence/_migrations/_up_to_3_worker.py new file mode 100644 index 00000000000..11adae2018b --- /dev/null +++ b/robot-server/robot_server/persistence/_migrations/_up_to_3_worker.py @@ -0,0 +1,111 @@ +"""Code that runs in a worker subprocess for the `up_to_3` migration.""" + + +# fmt: off + +# We keep a list of all the modules that this file imports +# so we can preload them when launching the subprocesses. +from types import ModuleType +_imports: "list[ModuleType]" = [] + +import contextlib # noqa: E402 +import pathlib # noqa: E402 +import typing # noqa: E402 +_imports.extend([contextlib, pathlib, typing]) + +import pydantic # noqa: E402 +import sqlalchemy # noqa: E402 +_imports.extend([pydantic, sqlalchemy]) + +from opentrons.protocol_engine import commands # noqa: E402 +from server_utils import sql_utils # noqa: E402 +_imports.extend([commands, sql_utils]) + +from robot_server.persistence._tables import schema_2, schema_3 # noqa: E402 +from robot_server.persistence import ( # noqa: E402 + _database, + legacy_pickle, + pydantic as pydantic_helpers +) +_imports.extend([schema_2, schema_3, _database, legacy_pickle, pydantic_helpers]) + +# fmt: on + + +imports: typing.List[str] = [m.__name__ for m in _imports] +"""The names of all modules imported by this module, e.g. "foo.bar.baz".""" + + +def migrate_commands_for_run( + source_db_file: pathlib.Path, + dest_db_file: pathlib.Path, + run_id: str, + # This is a multiprocessing.Lock, which can't be a type annotation for some reason. + lock: typing.ContextManager[object], +) -> None: + """Perform the schema 2->3 migration for a single run's commands. + + See the `up_to_3` migration for background. + + Args: + source_db_file: The SQLite database file to migrate from. + dest_db_file: The SQLite database file to migrate into. Assumed to have all the + proper tables set up already. + run_id: Which run's commands to migrate. + lock: A lock to hold while accessing the database. Concurrent access would be + safe in the sense that SQLite would always provide isolation. But, when + there are conflicts, we'd have to deal with SQLite retrying transactions or + raising SQLITE_BUSY. A Python-level lock is simpler and more reliable. + """ + with contextlib.suppress( + # The format that we're migrating from is prone to bugs where our latest + # code can't read records created by older code. (See RSS-98). + # If that happens, it's better to silently drop the run than to fail the + # whole migration. + # + # TODO(mm, 2024-02-14): Log these somehow. Logging is a little tricky from + # subprocesses. + Exception + ), _database.sql_engine_ctx( + source_db_file + ) as source_engine, _database.sql_engine_ctx( + dest_db_file + ) as dest_engine: + select_old_commands = sqlalchemy.select(schema_2.run_table.c.commands).where( + schema_2.run_table.c.id == run_id + ) + insert_new_command = sqlalchemy.insert(schema_3.run_command_table) + + with lock, source_engine.begin() as source_transaction: + old_commands_bytes: typing.Optional[bytes] = source_transaction.execute( + select_old_commands + ).scalar_one() + + old_commands: typing.List[typing.Dict[str, object]] = ( + legacy_pickle.loads(old_commands_bytes) if old_commands_bytes else [] + ) + + parsed_commands: typing.Iterable[commands.Command] = ( + pydantic.parse_obj_as( + commands.Command, # type: ignore[arg-type] + c, + ) + for c in old_commands + ) + + new_command_rows = [ + { + "run_id": run_id, + "index_in_run": index_in_run, + "command_id": parsed_command.id, + "command": pydantic_helpers.pydantic_to_json(parsed_command), + } + for index_in_run, parsed_command in enumerate(parsed_commands) + ] + + with lock, dest_engine.begin() as dest_transaction: + if len(new_command_rows) > 0: + # This needs to be guarded by a len>0 check because if the list is empty, + # SQLAlchemy misinterprets this as inserting a single row with all default + # values. + dest_transaction.execute(insert_new_command, new_command_rows) diff --git a/robot-server/robot_server/persistence/_migrations/up_to_3.py b/robot-server/robot_server/persistence/_migrations/up_to_3.py index a6fb8afacdb..906cdf70dd5 100644 --- a/robot-server/robot_server/persistence/_migrations/up_to_3.py +++ b/robot-server/robot_server/persistence/_migrations/up_to_3.py @@ -2,7 +2,7 @@ Summary of changes from schema 2: -- Run commands were formerly stored as monolithic blobs in the `run` table, +- Run commands were formerly stored as monolithic blobs in the `run.commands` column, with each row storing an entire list. This has been split out into a new `run_command` table, where each individual command gets its own row. @@ -16,24 +16,27 @@ since the updated `analysis.completed_analysis` (see above) replaces it. """ - +import multiprocessing from contextlib import ExitStack from pathlib import Path -from typing import Any, Dict, Iterable, List +from logging import getLogger +from typing import List -from opentrons.protocol_engine import Command, StateSummary +from opentrons.protocol_engine import StateSummary import pydantic import sqlalchemy from ..pydantic import pydantic_to_json from .._database import ( - create_schema_2_sql_engine, - create_schema_3_sql_engine, + sql_engine_ctx, sqlite_rowid, ) from .._folder_migrator import Migration from .._tables import schema_2, schema_3 from ._util import copy_rows_unmodified, copy_if_exists, copytree_if_exists +from . import up_to_2 + +from . import _up_to_3_worker # TODO: Define a single source of truth somewhere for these paths. @@ -42,6 +45,9 @@ _DB_FILE = "robot_server.db" +_log = getLogger(__name__) + + class MigrationUpTo3(Migration): # noqa: D101 def migrate(self, source_dir: Path, dest_dir: Path) -> None: """Migrate the persistence directory from schema 2 to 3.""" @@ -52,19 +58,37 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: source_dir / _PROTOCOLS_DIRECTORY, dest_dir / _PROTOCOLS_DIRECTORY ) + source_db_file = source_dir / _DB_FILE + dest_db_file = dest_dir / _DB_FILE + with ExitStack() as exit_stack: - # If the source is schema 0 or 1, this will migrate it to 2 in-place. - source_db = create_schema_2_sql_engine(source_dir / _DB_FILE) - exit_stack.callback(source_db.dispose) + source_engine = exit_stack.enter_context(sql_engine_ctx(source_db_file)) + schema_2.metadata.create_all(source_engine) + up_to_2.migrate(source_engine) + + dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file)) + schema_3.metadata.create_all(dest_engine) + + source_transaction = exit_stack.enter_context(source_engine.begin()) + dest_transaction = exit_stack.enter_context(dest_engine.begin()) - dest_db = create_schema_3_sql_engine(dest_dir / _DB_FILE) - exit_stack.callback(dest_db.dispose) + _migrate_db_excluding_commands(source_transaction, dest_transaction) - with source_db.begin() as source_transaction, dest_db.begin() as dest_transaction: - _migrate_db(source_transaction, dest_transaction) + # Get the run IDs *after* migrating runs, in case any runs got dropped. + run_ids = _get_run_ids(schema_3_transaction=dest_transaction) + _migrate_db_commands(source_db_file, dest_db_file, run_ids) -def _migrate_db( + +def _get_run_ids(*, schema_3_transaction: sqlalchemy.engine.Connection) -> List[str]: + return ( + schema_3_transaction.execute(sqlalchemy.select(schema_3.run_table.c.id)) + .scalars() + .all() + ) + + +def _migrate_db_excluding_commands( source_transaction: sqlalchemy.engine.Connection, dest_transaction: sqlalchemy.engine.Connection, ) -> None: @@ -81,7 +105,7 @@ def _migrate_db( dest_transaction, ) - _migrate_run_table( + _migrate_run_table_excluding_commands( source_transaction, dest_transaction, ) @@ -95,81 +119,99 @@ def _migrate_db( ) -def _migrate_run_table( +def _migrate_run_table_excluding_commands( source_transaction: sqlalchemy.engine.Connection, dest_transaction: sqlalchemy.engine.Connection, ) -> None: - select_old_runs = sqlalchemy.select(schema_2.run_table).order_by(sqlite_rowid) + select_old_runs = sqlalchemy.select( + schema_2.run_table.c.id, + schema_2.run_table.c.created_at, + schema_2.run_table.c.protocol_id, + schema_2.run_table.c.state_summary, + # schema_2.run_table.c.commands deliberately omitted + schema_2.run_table.c.engine_status, + schema_2.run_table.c._updated_at, + ).order_by(sqlite_rowid) insert_new_run = sqlalchemy.insert(schema_3.run_table) - insert_new_command = sqlalchemy.insert(schema_3.run_command_table) - - for old_run_row in source_transaction.execute(select_old_runs).all(): - old_state_summary = old_run_row.state_summary - new_state_summary = ( - None - if old_run_row.state_summary is None - else pydantic_to_json( - pydantic.parse_obj_as(StateSummary, old_state_summary) - ) - ) - dest_transaction.execute( - insert_new_run, - id=old_run_row.id, - created_at=old_run_row.created_at, - protocol_id=old_run_row.protocol_id, - state_summary=new_state_summary, - engine_status=old_run_row.engine_status, - _updated_at=old_run_row._updated_at, - ) - old_commands: List[Dict[str, Any]] = old_run_row.commands or [] - pydantic_old_commands: Iterable[Command] = ( - pydantic.parse_obj_as( - Command, # type: ignore[arg-type] - c, + for old_row in source_transaction.execute(select_old_runs).all(): + try: + old_state_summary = old_row.state_summary + new_state_summary = ( + None + if old_row.state_summary is None + else pydantic_to_json( + pydantic.parse_obj_as(StateSummary, old_state_summary) + ) + ) + dest_transaction.execute( + insert_new_run, + id=old_row.id, + created_at=old_row.created_at, + protocol_id=old_row.protocol_id, + state_summary=new_state_summary, + engine_status=old_row.engine_status, + _updated_at=old_row._updated_at, + ) + except Exception: + # The format that we're migrating from is prone to bugs where our latest + # code can't read records created by older code. (See RSS-98). + # If that happens, it's better to silently drop the run than to fail the + # whole migration. + _log.warning( + f"Exception while migrating run {old_row.id}. Dropping it.", + exc_info=True, ) - for c in old_commands - ) - new_command_rows = [ - { - "run_id": old_run_row.id, - "index_in_run": index_in_run, - "command_id": pydantic_command.id, - "command": pydantic_to_json(pydantic_command), - } - for index_in_run, pydantic_command in enumerate(pydantic_old_commands) - ] - # Insert all the commands for this run in one go, to avoid the overhead of - # separate statements, and since we had to bring them all into memory at once - # in order to parse them anyway. - if len(new_command_rows) > 0: - # This needs to be guarded by a len>0 check because if the list is empty, - # SQLAlchemy misinterprets this as inserting a single row with all default - # values. - dest_transaction.execute(insert_new_command, new_command_rows) def _migrate_analysis_table( - source_connection: sqlalchemy.engine.Connection, - dest_connection: sqlalchemy.engine.Connection, + source_transaction: sqlalchemy.engine.Connection, + dest_transaction: sqlalchemy.engine.Connection, ) -> None: select_old_analyses = sqlalchemy.select(schema_2.analysis_table).order_by( sqlite_rowid ) insert_new_analysis = sqlalchemy.insert(schema_3.analysis_table) - for row in ( - # The table is missing an explicit sequence number column, so we need - # sqlite_rowid to retain order across this copy. - source_connection.execute(select_old_analyses).all() - ): - dest_connection.execute( + for old_row in source_transaction.execute(select_old_analyses).all(): + dest_transaction.execute( insert_new_analysis, # The new `completed_analysis` column has the data that used to be in # `completed_analysis_as_document`. The separate # `completed_analysis_as_document` column is dropped. - completed_analysis=row.completed_analysis_as_document, + completed_analysis=old_row.completed_analysis_as_document, # The remaining columns are unchanged: - id=row.id, - protocol_id=row.protocol_id, - analyzer_version=row.analyzer_version, + id=old_row.id, + protocol_id=old_row.protocol_id, + analyzer_version=old_row.analyzer_version, + ) + + +def _migrate_db_commands( + source_db_file: Path, dest_db_file: Path, run_ids: List[str] +) -> None: + """Migrate the run commands stored in the database. + + Because there are potentially tens or hundreds of thousands of commands in total, + this is the most computationally expensive part of the migration. We distribute + the work across subprocesses. Each subprocess extracts, migrates, and inserts + all of the commands for a single run. + """ + mp = multiprocessing.get_context("forkserver") + mp.set_forkserver_preload(_up_to_3_worker.imports) + + manager = mp.Manager() + lock = manager.Lock() + + with mp.Pool( + # One worker per core of the OT-2's Raspberry Pi. + # We're compute-bound, so more workers would just thrash. + # + # Napkin math for the memory footprint: + # Suppose a very large run has ~10 MB of commands (see e.g. RQA-443). + # We're limiting this to 4 runs at a time, so 40 MB, which should be fine. + processes=4 + ) as pool: + pool.starmap( + _up_to_3_worker.migrate_commands_for_run, + ((source_db_file, dest_db_file, run_id, lock) for run_id in run_ids), ) diff --git a/robot-server/robot_server/persistence/_tables/schema_2.py b/robot-server/robot_server/persistence/_tables/schema_2.py index fc23e96b5d7..3537757845e 100644 --- a/robot-server/robot_server/persistence/_tables/schema_2.py +++ b/robot-server/robot_server/persistence/_tables/schema_2.py @@ -105,7 +105,7 @@ # column added in schema v1 sqlalchemy.Column( "commands", - sqlalchemy.PickleType(pickler=legacy_pickle, protocol=PICKLE_PROTOCOL_VERSION), + sqlalchemy.LargeBinary, nullable=True, ), # column added in schema v1 diff --git a/robot-server/robot_server/persistence/legacy_pickle.py b/robot-server/robot_server/persistence/legacy_pickle.py index 0ad36054cbc..36d68a1968a 100644 --- a/robot-server/robot_server/persistence/legacy_pickle.py +++ b/robot-server/robot_server/persistence/legacy_pickle.py @@ -15,7 +15,7 @@ # unknown types. dumps as dumps, ) -from typing import Dict, List +from typing import Any, Dict, List _log = getLogger(__name__) @@ -69,7 +69,7 @@ def find_class(self, module: str, name: str) -> object: # noqa: D102 return super().find_class(module, name) -def loads(data: bytes) -> object: +def loads(data: bytes) -> Any: """Drop-in replacement for `pickle.loads` that uses our custom unpickler.""" return LegacyUnpickler(BytesIO(data)).load() diff --git a/robot-server/tests/conftest.py b/robot-server/tests/conftest.py index c3a225d7571..3014c922faa 100644 --- a/robot-server/tests/conftest.py +++ b/robot-server/tests/conftest.py @@ -40,7 +40,7 @@ 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_schema_3_sql_engine +from robot_server.persistence import get_sql_engine, metadata, sql_engine_ctx from robot_server.health.router import ComponentVersions, get_versions test_router = routing.APIRouter() @@ -393,6 +393,6 @@ def clear_custom_tiprack_def_dir() -> Iterator[None]: def sql_engine(tmp_path: Path) -> Generator[SQLEngine, None, None]: """Return a set-up database to back the store.""" db_file_path = tmp_path / "test.db" - sql_engine = create_schema_3_sql_engine(db_file_path) - yield sql_engine - sql_engine.dispose() + with sql_engine_ctx(db_file_path) as engine: + metadata.create_all(engine) + yield engine From 14cfb6e90b180c6dcfb5dd83ecd67f360201e012 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 14 Feb 2024 12:37:43 -0500 Subject: [PATCH 24/24] perf(robot-server): Remove slow re-exports from __init__.py and protocols/__init__.py (#14480) --- robot-server/Makefile | 2 +- robot-server/opentrons-robot-server.service | 2 +- robot-server/robot_server/__init__.py | 6 ------ robot-server/robot_server/app.py | 10 ++++++++++ robot-server/robot_server/protocols/__init__.py | 15 --------------- robot-server/robot_server/router.py | 2 +- robot-server/robot_server/runs/engine_store.py | 2 +- .../robot_server/runs/router/base_router.py | 6 +++--- .../robot_server/runs/run_data_manager.py | 2 +- robot-server/robot_server/runs/run_store.py | 2 +- robot-server/tests/conftest.py | 2 +- robot-server/tests/integration/dev_server.py | 2 +- robot-server/tests/runs/router/conftest.py | 2 +- .../tests/runs/router/test_base_router.py | 6 +++--- robot-server/tests/runs/test_engine_store.py | 2 +- robot-server/tests/runs/test_run_data_manager.py | 2 +- .../tests/service/legacy/routers/test_settings.py | 2 +- 17 files changed, 28 insertions(+), 39 deletions(-) create mode 100644 robot-server/robot_server/app.py diff --git a/robot-server/Makefile b/robot-server/Makefile index 57cb578b56d..e8590254b29 100755 --- a/robot-server/Makefile +++ b/robot-server/Makefile @@ -71,7 +71,7 @@ clean_all_cmd = $(clean_cmd) dist # probably POSIX-only. dev_port ?= "31950" dev_host ?= "localhost" -run_dev ?= uvicorn "robot_server:app" --host $(dev_host) --port $(dev_port) --ws wsproto --lifespan on --reload +run_dev ?= uvicorn "robot_server.app:app" --host $(dev_host) --port $(dev_port) --ws wsproto --lifespan on --reload .PHONY: all all: clean sdist wheel diff --git a/robot-server/opentrons-robot-server.service b/robot-server/opentrons-robot-server.service index 095c4fb39bb..48648658cec 100644 --- a/robot-server/opentrons-robot-server.service +++ b/robot-server/opentrons-robot-server.service @@ -18,7 +18,7 @@ Type=notify # /run/aiohttp.sock matches where our reverse proxy expects to find us. # It refers to aiohttp even though this server doesn't use that framework # for historical reasons. -ExecStart=uvicorn robot_server:app --uds /run/aiohttp.sock --ws wsproto --lifespan on +ExecStart=uvicorn robot_server.app:app --uds /run/aiohttp.sock --ws wsproto --lifespan on Environment=OT_SMOOTHIE_ID=AMA Environment=RUNNING_ON_PI=true diff --git a/robot-server/robot_server/__init__.py b/robot-server/robot_server/__init__.py index 4d5dcbf1ddc..329cf0c1e83 100644 --- a/robot-server/robot_server/__init__.py +++ b/robot-server/robot_server/__init__.py @@ -2,9 +2,3 @@ This server provides the main control interface for an Opentrons robot. """ - -from .app_setup import app - -__all__ = [ - "app", -] diff --git a/robot-server/robot_server/app.py b/robot-server/robot_server/app.py new file mode 100644 index 00000000000..4a229be1abd --- /dev/null +++ b/robot-server/robot_server/app.py @@ -0,0 +1,10 @@ +"""The public export of the server's ASGI app object. + +For import speed, we do this from a dedicated file instead of from the top-level +__init__.py. We want worker processes and tests to be able to import specific things +deep in robot_server without having to import this ASGI app and all of its dependencies. +""" + +from .app_setup import app + +__all__ = ["app"] diff --git a/robot-server/robot_server/protocols/__init__.py b/robot-server/robot_server/protocols/__init__.py index 34bdaaebe68..60f3ae5dc5a 100644 --- a/robot-server/robot_server/protocols/__init__.py +++ b/robot-server/robot_server/protocols/__init__.py @@ -1,16 +1 @@ """Protocol file upload and management.""" -from .router import protocols_router, ProtocolNotFound -from .dependencies import get_protocol_store -from .protocol_store import ProtocolStore, ProtocolResource, ProtocolNotFoundError - -__all__ = [ - # main protocols router - "protocols_router", - # common error response details - "ProtocolNotFound", - # protocol state management - "get_protocol_store", - "ProtocolStore", - "ProtocolResource", - "ProtocolNotFoundError", -] diff --git a/robot-server/robot_server/router.py b/robot-server/robot_server/router.py index 4739c4d84ce..2398e9fe161 100644 --- a/robot-server/robot_server/router.py +++ b/robot-server/robot_server/router.py @@ -11,7 +11,7 @@ from .instruments import instruments_router from .maintenance_runs.router import maintenance_runs_router from .modules import modules_router -from .protocols import protocols_router +from .protocols.router import protocols_router from .robot.router import robot_router from .runs import runs_router from .service.labware.router import router as labware_router diff --git a/robot-server/robot_server/runs/engine_store.py b/robot-server/robot_server/runs/engine_store.py index 350d5bc694c..d938fbbbe25 100644 --- a/robot-server/robot_server/runs/engine_store.py +++ b/robot-server/robot_server/runs/engine_store.py @@ -31,7 +31,7 @@ create_protocol_engine, ) -from robot_server.protocols import ProtocolResource +from robot_server.protocols.protocol_store import ProtocolResource from opentrons.protocol_engine.types import DeckConfigurationType diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index d7c1ea1bc59..3edd9a342ba 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -27,12 +27,12 @@ PydanticResponse, ) -from robot_server.protocols import ( +from robot_server.protocols.dependencies import get_protocol_store +from robot_server.protocols.protocol_store import ( ProtocolStore, - ProtocolNotFound, ProtocolNotFoundError, - get_protocol_store, ) +from robot_server.protocols.router import ProtocolNotFound from ..run_models import RunNotFoundError from ..run_auto_deleter import RunAutoDeleter diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index be62c7b704f..acf0ddec6c4 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -13,7 +13,7 @@ Command, ) -from robot_server.protocols import ProtocolResource +from robot_server.protocols.protocol_store import ProtocolResource from robot_server.service.task_runner import TaskRunner from robot_server.service.notifications import RunsPublisher diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 40e59143e76..aa65ce19704 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -20,7 +20,7 @@ sqlite_rowid, ) from robot_server.persistence.pydantic import json_to_pydantic, pydantic_to_json -from robot_server.protocols import ProtocolNotFoundError +from robot_server.protocols.protocol_store import ProtocolNotFoundError from robot_server.service.notifications import RunsPublisher from .action_models import RunAction, RunActionType diff --git a/robot-server/tests/conftest.py b/robot-server/tests/conftest.py index 3014c922faa..f3a5ce2761e 100644 --- a/robot-server/tests/conftest.py +++ b/robot-server/tests/conftest.py @@ -36,7 +36,7 @@ from opentrons.protocol_api import labware from opentrons.types import Point, Mount -from robot_server import app +from robot_server.app import app 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 diff --git a/robot-server/tests/integration/dev_server.py b/robot-server/tests/integration/dev_server.py index ab774cc750b..f549a4752e8 100644 --- a/robot-server/tests/integration/dev_server.py +++ b/robot-server/tests/integration/dev_server.py @@ -79,7 +79,7 @@ def start(self) -> None: "robot_server", "-m", "uvicorn", - "robot_server:app", + "robot_server.app:app", "--host", "localhost", "--port", diff --git a/robot-server/tests/runs/router/conftest.py b/robot-server/tests/runs/router/conftest.py index f7d1f0fead6..96b0bb578e7 100644 --- a/robot-server/tests/runs/router/conftest.py +++ b/robot-server/tests/runs/router/conftest.py @@ -2,7 +2,7 @@ import pytest from decoy import Decoy -from robot_server.protocols import ProtocolStore +from robot_server.protocols.protocol_store import ProtocolStore from robot_server.runs.run_auto_deleter import RunAutoDeleter from robot_server.runs.run_store import RunStore from robot_server.runs.engine_store import EngineStore diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index 0abe559b843..c4ba00657c0 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/robot-server/tests/runs/router/test_base_router.py @@ -17,10 +17,10 @@ ResourceLink, ) -from robot_server.protocols import ( - ProtocolStore, - ProtocolResource, +from robot_server.protocols.protocol_store import ( ProtocolNotFoundError, + ProtocolResource, + ProtocolStore, ) from robot_server.runs.run_auto_deleter import RunAutoDeleter diff --git a/robot-server/tests/runs/test_engine_store.py b/robot-server/tests/runs/test_engine_store.py index bd8ef9b3678..1bf74632139 100644 --- a/robot-server/tests/runs/test_engine_store.py +++ b/robot-server/tests/runs/test_engine_store.py @@ -18,7 +18,7 @@ ) from opentrons.protocol_reader import ProtocolReader, ProtocolSource -from robot_server.protocols import ProtocolResource +from robot_server.protocols.protocol_store import ProtocolResource from robot_server.runs.engine_store import ( EngineStore, EngineConflictError, diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index bc8f9ad16cb..cabaa09ae05 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -21,7 +21,7 @@ LabwareOffset, ) -from robot_server.protocols import ProtocolResource +from robot_server.protocols.protocol_store import ProtocolResource from robot_server.runs.engine_store import EngineStore, EngineConflictError from robot_server.runs.run_data_manager import RunDataManager, RunNotCurrentError from robot_server.runs.run_models import Run, RunNotFoundError diff --git a/robot-server/tests/service/legacy/routers/test_settings.py b/robot-server/tests/service/legacy/routers/test_settings.py index 58bcfefffe0..27a930617fd 100644 --- a/robot-server/tests/service/legacy/routers/test_settings.py +++ b/robot-server/tests/service/legacy/routers/test_settings.py @@ -18,7 +18,7 @@ from opentrons_shared_data.robot.dev_types import RobotTypeEnum -from robot_server import app +from robot_server.app import app from robot_server.deck_configuration.fastapi_dependencies import ( get_deck_configuration_store_failsafe, )