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

feat(robot-server): Persist /labwareOffsets storage across reboots #17232

Open
wants to merge 7 commits into
base: edge
Choose a base branch
from

Conversation

SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented Jan 9, 2025

Overview

Closes EXEC-1015.

Test plan and Hands on testing

We're covered by automated tests.

Changelog

  • Add a new labware_offset SQL table.
  • Make LabwareOffsetStore to use it.

Review requests

EXEC-1015 has a guideline:

Follow database archival practices and have a validity marking rather than an expectation of deletion when a row becomes invalid (this should allow persistent reference to IDs)

The way I'm implementing that here is:

  • HTTP DELETE requests do an actual SQL DELETE.
  • There is a SQL boolean column, active, to represent the validity marking.
    • It is currently always set to TRUE, and there is currently no way of reading it.
    • I'm imagining that future work could add HTTP APIs like PATCH /labwareOffsets/{id} { "active": false } and GET /labwareOffsets?active=true.

Does all of that sound right?

Risk assessment

Low. This code is currently unused.

If we need to make any changes to the SQL schema, it's possible to do later but kind of annoying, so we should scrutinize it here, to the extent that we can.

Comment on lines +390 to +392
# Whether this record is "active", i.e. whether it should be considered as a
# candidate to apply to runs and affect actual robot motion:
sqlalchemy.Column("active", sqlalchemy.Boolean, nullable=False),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is active a good name for this? Maybe valid, hidden, archived, ...?

@SyntaxColoring SyntaxColoring requested a review from a team January 9, 2025 20:09
@SyntaxColoring SyntaxColoring marked this pull request as ready for review January 9, 2025 20:09
@SyntaxColoring SyntaxColoring requested a review from a team as a code owner January 9, 2025 20:09
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.

1 participant