Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(opentrons-shared-data): fix performance module not being recognized #14990

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

DerekMaggio
Copy link
Contributor

@DerekMaggio DerekMaggio commented Apr 23, 2024

Overview

Fixes https://opentrons.atlassian.net/browse/RQA-2623

performance directory was missing an __init__.py

Test Plan

  • I added a test just to import the module and create one of the objects inside the dev_types.py. The issue is that the tests don't run against the built version of the package. I built the app and everything imports correctly
  • I cannot test that opentrons_shared_data is being utilized correctly on the robot until I have a built buildroot image. Because currently my robot and app have different versions (due to the dev build) so the app will not let me trigger an analysis.
  • I instead pushed opentrons-shared-data to my robot and verified that the performance module existed in /usr/lib/python3.10/site-packages/opentrons_shared_data. But I can't test that the imports actually work until I have the system image and the app together

Changelog

  • Added __init__.py to performance directory to tell python to import it as a module
  • Reorganized performance_helpers.py to not have an import error

Review requests

  • Nothing to block this fix, but what should be done to make sure this doesn't happen again? This is a weird packaging thing that doesn't show up when running dev or CI testing.
  • I wonder if there is a smoke test we can perform automatically just to make sure everything imports correctly? Running opentrons.simulate against the actual built package would have caught this

Risk assessment

Medium, I mean I can't break it any worse than I already did

@DerekMaggio DerekMaggio changed the title test: verify shared-data includes performance module fix(opentrons-shared-data): fix performance module not being recognized Apr 23, 2024
@DerekMaggio DerekMaggio requested a review from a team April 23, 2024 22:56
@DerekMaggio DerekMaggio marked this pull request as ready for review April 23, 2024 22:57
@DerekMaggio DerekMaggio requested a review from a team as a code owner April 23, 2024 22:57
@DerekMaggio DerekMaggio requested a review from y3rsh April 23, 2024 22:57
@y3rsh y3rsh merged commit 40db9c5 into edge Apr 24, 2024
47 checks passed
@y3rsh y3rsh deleted the RQA-2623-perf-metrics-data-not-being-found branch April 24, 2024 00:28
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…ed (#14990)

# Overview

Fixes https://opentrons.atlassian.net/browse/RQA-2623

performance directory was missing an `__init__.py`

# Test Plan

- I added a test just to import the module and create one of the objects
inside the dev_types.py. The issue is that the tests don't run against
the built version of the package. I built the app and everything imports
correctly
- I cannot test that opentrons_shared_data is being utilized correctly
on the robot until I have a built buildroot image. Because currently my
robot and app have different versions (due to the dev build) so the app
will not let me trigger an analysis.
- I instead pushed opentrons-shared-data to my robot and verified that
the performance module existed in
`/usr/lib/python3.10/site-packages/opentrons_shared_data`. But I can't
test that the imports actually work until I have the system image and
the app together

# Changelog

- Added `__init__.py` to performance directory to tell python to import
it as a module
- Reorganized performance_helpers.py to not have an import error

# Review requests

- Nothing to block this fix, but what should be done to make sure this
doesn't happen again? This is a weird packaging thing that doesn't show
up when running dev or CI testing.
- I wonder if there is a smoke test we can perform automatically just to
make sure everything imports correctly? Running opentrons.simulate
against the actual built package would have caught this


# Risk assessment

Medium, I mean I can't break it any worse than I already did
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants