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

Remove the "resource_snapshot" type from the resource_snapshot_types Enum table #474

Open
1 of 3 tasks
jkglasbrenner opened this issue May 23, 2024 · 1 comment · May be fixed by #703
Open
1 of 3 tasks

Remove the "resource_snapshot" type from the resource_snapshot_types Enum table #474

jkglasbrenner opened this issue May 23, 2024 · 1 comment · May be fixed by #703
Assignees
Labels
bug Something isn't working

Comments

@jkglasbrenner
Copy link
Collaborator

jkglasbrenner commented May 23, 2024

This will prevent the creation of a "bare" ResourceSnapshot entry with type "resource_snapshot" in the database. All snapshots are supposed to be an inherited type, so we want to prevent this at the data integrity level.

A couple hints for this. First, to generate a blank migration script (is necessary here), use

dioptra-db revision -m "<Migration Script Message Goes Here>"

That will create a new migration script file in src/dioptra/restapi/db/alembic/versions/.

A migration script that does something similar to what we need here is https://github.com/usnistgov/dioptra/blob/dev/src/dioptra/restapi/db/alembic/versions/10f9e72e72aa_add_readonly_resource_lock_type_to_db_.py. However, there are a couple of key differences:

  1. The linked migration script is adding a record to the Enum table. This change requires deleting an existing value.
  2. The linked migration script does not have a downgrade script. This change requires us to have both a working upgrade and downgrade script. Upgrade deletes the "resource_snapshot" record, downgrade inserts it back into the table.

The main information to take from the linked migration script is that it shows how you can set up the migration ORM models to handle CRUD operations on the table. Note that the migration ORM models are supposed to be standalone, we should not import the models under src/dioptra/restapi/db/models in a migration script.

This migration script has a little more complexity, and shows the need for both an upgrade and downgrade: https://github.com/usnistgov/dioptra/blob/dev/src/dioptra/restapi/db/alembic/versions/6a75ede23821_update_entry_point_parameter_types_list.py

Definition of Done

  • The "resource_snapshot" enum entry is removed from the table, and a new migration script is created
  • All existing tests pass
  • The bug fix is merged into dev
@jkglasbrenner jkglasbrenner added the bug Something isn't working label May 23, 2024
@jkglasbrenner jkglasbrenner self-assigned this May 23, 2024
@JustKuzya
Copy link
Collaborator

To test you need to setup flask environment, but not start flask yet

To test upgrade run:

dioptra-db upgrade --revision 0ca6eca33569

Or (very unlikely) if on version 6a75ede23821 and the only newer version is 0ca6eca33569

dioptra-db upgrade

To test downgrade run:

dioptra-db downgrade 6a75ede23821

@JustKuzya JustKuzya linked a pull request Dec 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants