aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
412 stars 184 forks source link

`SqliteDosStorage`: Make the migrator compatible with SQLite #6429

Open sphuber opened 1 month ago

sphuber commented 1 month ago

The majority of the SqliteDosStorage piggy-backs off of the PsqlDosStorage plugin. It also uses the PsqlDosMigrator as-is to perform the database migrations. This is not safe however, as PostgreSQL and SQLite do not have exactly the same syntax.

An example is the main_0002 revision which was added to drop the hashes of certain nodes. This uses the #- operator which is JSONB specific syntax of PostgreSQL and is not supported by SQLite. Since this migration was added before the SqliteDosStorage plugin was added, this has never caused a problems as all profiles would be new, would not have any nodes and therefore the SQL code of the migration would not actually be executed.

In preparation for any future migrations that may need to be added, the SqliteDosStorage now uses the SqliteDosMigrator. This subclasses the PsqlDosMigrator as it can still use most of the functionality, but it changes a few critical things. Most notably the location of the schema versions which now are kept individually and are no longer lent from the core.psql_dos plugin. The exception is the original schema which is identical and so is symlinked.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 83.67347% with 8 lines in your changes missing coverage. Please review.

Project coverage is 77.86%. Comparing base (ef60b66) to head (c2e565a). Report is 30 commits behind head on main.

Files Patch % Lines
src/aiida/storage/sqlite_dos/backend.py 84.79% 7 Missing :warning:
src/aiida/storage/sqlite_zip/migrations/env.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6429 +/- ## ========================================== + Coverage 77.51% 77.86% +0.36% ========================================== Files 560 563 +3 Lines 41444 41833 +389 ========================================== + Hits 32120 32571 +451 + Misses 9324 9262 -62 ``` | [Flag](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6429/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [presto](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | `73.13% <83.68%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

danielhollas commented 3 weeks ago

@sphuber I punted on reviewing this since afaik it's not a blocker for 2.6.0 release. LMK I case it became a priority, otherwise I'll get to it after 20th June, unless someone else reviews first.

sphuber commented 3 weeks ago

Thanks @danielhollas . It is not critical for the release. Enjoy your holidays!