MarquezProject / marquez

Collect, aggregate, and visualize a data ecosystem's metadata
https://marquezproject.ai
Apache License 2.0
1.78k stars 320 forks source link

Repurpose db-migrate to run all pending migrations #2936

Closed davidjgoss closed 1 month ago

davidjgoss commented 1 month ago

Problem

There was no Marquez command to run all pending Flyway migrations. This is needed in the context of wanting to run migrations in a short-lived container ahead of the application deployment, so migrations are not coupled to startup. The db-migrate command sort of looks like it should do this, but actually only supports running a single specific Java migration which is from ~20 versions ago.

Solution

This PR repurposes db-migrate to run all pending migrations.

The original intent was to leave the existing functionality around the V57 migration in place and support omitting the version argument to run all pending migrations. However, this was dependent on the command remaining an EnvironmentCommand which caused its own problem, because such a command by nature requires the application's run method to have been called, which contains the migration-on-startup logic. So the scenario would go:

  1. Set migrateOnStartup: false in the config
  2. Execute marquez.jar db-migrate my-config.yml
  3. MarquezApp::run is called by Dropwizard
  4. migrateDbOrError detects pending migrations and throws because they ostensibly aren't about to run
  5. We never make it to the db-migrate command

So instead we have reworked the command to be a ConfiguredCommand which only requires the resolved configuration and bootstrap object, and can be run without the application's run method ever being touched.

One-line summary: Repurpose db-migrate to run all pending migrations

Checklist

netlify[bot] commented 1 month ago

Deploy Preview for peppy-sprite-186812 canceled.

Name Link
Latest commit 5bbb565e184bdc8c664e99d3a5b8026b5d283a85
Latest deploy log https://app.netlify.com/sites/peppy-sprite-186812/deploys/6716c92e00f36600088b9af8
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.16%. Comparing base (b7277d7) to head (5bbb565). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
api/src/main/java/marquez/db/DbMigration.java 0.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2936 +/- ## ============================================ + Coverage 81.12% 81.16% +0.03% Complexity 1506 1506 ============================================ Files 268 268 Lines 7365 7363 -2 Branches 329 329 ============================================ + Hits 5975 5976 +1 + Misses 1229 1226 -3 Partials 161 161 ```

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