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

Replace DB package tables by sip and aip tables #1121

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

jraddaoui
Copy link
Collaborator

@jraddaoui jraddaoui commented Feb 10, 2025

Stop using package across the persistence layers. Use SIP on ingest
and AIP on storage renaming DB tables, types and enums. The use of
package still happens on the API design/spec and the dashboard, these
will be modified in following PRs.

  • Allow generating migrations for both databases using ariga.io/atlas.
  • Add atlas-hash Make rule to re-generate sum file in migration dirs.
  • Update ingest database, ent schema and client:
    • Rename package table to sip.
    • Adapt preservation_action table relation.
    • Generate use_atlas migration to match schema from ent/Atlas.
    • Rename package related functions.
  • Update storage database, ent schema and client:
    • Rename package table to aip.
    • Rename package related functions.
  • Rename datatype SIP and PreservationAction.SIPID.
  • Rename SIP statuses and types.
  • Rename AIP statuses.

Refs #1117.

@jraddaoui jraddaoui self-assigned this Feb 10, 2025
@jraddaoui jraddaoui force-pushed the dev/issue-1117-db-schema branch from 120cf64 to dd6d7dc Compare February 10, 2025 21:00
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

Attention: Patch coverage is 69.12752% with 92 lines in your changes missing coverage. Please review.

Project coverage is 54.72%. Comparing base (4731e95) to head (5f10106).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/storage/persistence/telemetry.go 0.00% 24 Missing ⚠️
cmd/migrate/main.go 0.00% 23 Missing ⚠️
internal/package_/package_.go 16.66% 15 Missing ⚠️
internal/persistence/telemetry.go 0.00% 12 Missing ⚠️
internal/storage/types/aip_status.go 87.93% 6 Missing and 1 partial ⚠️
internal/datatypes/sip.go 88.00% 2 Missing and 1 partial ⚠️
internal/package_/preservation_action.go 0.00% 2 Missing ⚠️
internal/storage/workflows/move.go 0.00% 0 Missing and 2 partials ⚠️
internal/package_/goa.go 66.66% 1 Missing ⚠️
internal/persistence/ent/client/sip.go 97.43% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1121   +/-   ##
=======================================
  Coverage   54.71%   54.72%           
=======================================
  Files         105      105           
  Lines        7713     7719    +6     
=======================================
+ Hits         4220     4224    +4     
- Misses       3231     3234    +3     
+ Partials      262      261    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jraddaoui jraddaoui force-pushed the dev/issue-1117-db-schema branch from dd6d7dc to 04267c8 Compare February 11, 2025 04:34
@jraddaoui jraddaoui changed the title WIP: Update DBs and clients for ingest and storage Replace DB package tables by sip and aip tables Feb 11, 2025
@jraddaoui jraddaoui marked this pull request as ready for review February 11, 2025 04:36
Copy link
Collaborator

@djjuhasz djjuhasz left a comment

Choose a reason for hiding this comment

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

Whew, finished the review. 😅

It looks great overall @jraddaoui.

In the database migrations it would be nice to avoid updating all of the column definitions, as I don't think they've actually changed, but that may just be a quirk of Atlas.

I've flagged a number of minor renaming issues, but I don't think there's any serious blockers.

@scollazo
Copy link
Contributor

Quick question, are migrations applied by the app each time it starts, or should a migration command be run before the app starts?

@jraddaoui
Copy link
Collaborator Author

Quick question, are migrations applied by the app each time it starts, or should a migration command be run before the app starts?

In this project we use versioned migrations even if they are applied on the application start, so there is no need of an extra command. The migrate config value for both databases must be set to true for that to happen though.

@jraddaoui jraddaoui force-pushed the dev/issue-1117-db-schema branch from 04267c8 to 4e6a371 Compare February 14, 2025 15:53
Stop using package across the persistence layers. Use SIP on ingest
and AIP on storage renaming DB tables, types and enums. The use of
package still happens on the API design/spec and the dashboard, these
will be modified in following PRs.

- Allow generating migrations for both databases using ariga.io/atlas.
- Add `atlas-hash` Make rule to re-generate sum file in migration dirs.
- Update ingest database, ent schema and client:
  - Rename `package` table to `sip`.
  - Adapt `preservation_action` table relation.
  - Generate `use_atlas` migration to match schema from ent/Atlas.
  - Rename package related functions.
- Update storage database, ent schema and client:
  - Rename `package` table to `aip`.
  - Rename package related functions.
- Rename datatype SIP and PreservationAction.SIPID.
- Rename SIP statuses and types.
- Rename AIP statuses.
@jraddaoui jraddaoui force-pushed the dev/issue-1117-db-schema branch from 4e6a371 to 5f10106 Compare February 20, 2025 09:05
@jraddaoui jraddaoui merged commit 5f10106 into main Feb 20, 2025
14 checks passed
@jraddaoui jraddaoui deleted the dev/issue-1117-db-schema branch February 20, 2025 09:12
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.

3 participants