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

Repurpose db-migrate to run all pending migrations #2936

Merged
merged 2 commits into from
Oct 22, 2024
Merged

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Oct 21, 2024

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

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've included a one-line summary of your change for the CHANGELOG.md (Depending on the change, this may not be necessary).
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

@boring-cyborg boring-cyborg bot added the api API layer changes label Oct 21, 2024
Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for peppy-sprite-186812 canceled.

Name Link
🔨 Latest commit 5bbb565
🔍 Latest deploy log https://app.netlify.com/sites/peppy-sprite-186812/deploys/6716c92e00f36600088b9af8

Copy link

codecov bot commented Oct 21, 2024

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 ⚠️
Additional details and impacted files
@@             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              

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

@davidjgoss davidjgoss marked this pull request as ready for review October 21, 2024 15:50
@@ -25,15 +25,15 @@ public static void migrateDbOrError(@NonNull final DataSource source) {
public static void migrateDbOrError(
@NonNull final FlywayFactory flywayFactory,
@NonNull final DataSource source,
final boolean migrateDbOnStartup) {
final boolean migrateNow) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to make more sense in the context of db-migrate

@davidjgoss davidjgoss requested a review from wslulciuc October 21, 2024 21:40
Copy link
Member

@wslulciuc wslulciuc left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@wslulciuc wslulciuc merged commit fcbbc07 into main Oct 22, 2024
16 checks passed
@wslulciuc wslulciuc deleted the feat/db-migrate-all branch October 22, 2024 05:51
jonathanpmoraes pushed a commit to nubank/NuMarquez that referenced this pull request Feb 6, 2025
jonathanpmoraes pushed a commit to nubank/NuMarquez that referenced this pull request Feb 6, 2025
* Fixing data quality display. (MarquezProject#2937)

Signed-off-by: phixMe <[email protected]>

* Dataset Version call simplification (MarquezProject#2938)

* Fixing data quality display.

Signed-off-by: phixMe <[email protected]>

* Fixing dataset version calls.

Signed-off-by: phixMe <[email protected]>

---------

Signed-off-by: phixMe <[email protected]>

* feat: allow db-migrate without version (MarquezProject#2936)

Signed-off-by: David Goss <[email protected]>

* Display full `runID` and check icon when copied (MarquezProject#2940)

Signed-off-by: Willy Lulciuc <[email protected]>

* Deferred copy revert. (MarquezProject#2941)

Signed-off-by: phixMe <[email protected]>

* Long text handling (MarquezProject#2942)

* Deferred copy revert.

Signed-off-by: phixMe <[email protected]>

* Long text handling.

Signed-off-by: phixMe <[email protected]>

* Adding search back in.

Signed-off-by: phixMe <[email protected]>

---------

Signed-off-by: phixMe <[email protected]>

* Use project root for docker volume prefix (MarquezProject#2943)

Signed-off-by: Willy Lulciuc <[email protected]>

* fix: Correct SQL query pagination for DatasetVersion findAll method (MarquezProject#2945)

Signed-off-by: Alper İnan <[email protected]>
Signed-off-by: Alper <[email protected]>

* Update changelog for `0.50.0`

Signed-off-by: Willy Lulciuc <[email protected]>

* Replace `redoc-cli` with `redocly`

Signed-off-by: Willy Lulciuc <[email protected]>

* Prepare for release 0.50.0

Signed-off-by: Willy Lulciuc <[email protected]>

* Prepare next development version 0.51.0-SNAPSHOT

Signed-off-by: Willy Lulciuc <[email protected]>

* Templatize event time in `metadata.json` (MarquezProject#2946)

* Templatize event time in `metadata.json`

Signed-off-by: Willy Lulciuc <[email protected]>

* Use `metadata.template.json`

Signed-off-by: Willy Lulciuc <[email protected]>

---------

Signed-off-by: Willy Lulciuc <[email protected]>

* Update CHANGELOG.md

* Update `web/docs/demo.gif` (MarquezProject#2948)

Signed-off-by: Willy Lulciuc <[email protected]>

* fix(deps): update dependency io.openlineage:openlineage-java to v1.23.0 (MarquezProject#2907)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update dependency org.assertj:assertj-core to v3.26.3 (MarquezProject#2909)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Willy Lulciuc <[email protected]>

* fix(deps): update dependency org.postgresql:postgresql to v42.7.4 (MarquezProject#2912)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update dependency org.opensearch.client:opensearch-rest-client to v2.17.1 (MarquezProject#2911)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Willy Lulciuc <[email protected]>

* fix(deps): update dependency org.apache.commons:commons-lang3 to v3.17.0 (MarquezProject#2908)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Ignore `**/stats/**` (MarquezProject#2952)

Signed-off-by: Willy Lulciuc <[email protected]>

* Update compatibility for `0.50.0`

* fix(deps): update dependency org.opensearch.client:opensearch-java to v2.16.0 (MarquezProject#2910)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* `Dataset.currentVersionUuid` `->` `DatasetVersion.uuid` (MarquezProject#2954)

Signed-off-by: Willy Lulciuc <[email protected]>

* Update Events Page (MarquezProject#2955)

* Tuning the events page for longer events.

Signed-off-by: phixMe <[email protected]>

* Adding events file.

Signed-off-by: phixMe <[email protected]>

* Refetch jobs button.

Signed-off-by: phixMe <[email protected]>

* Refetch jobs button.

Signed-off-by: phixMe <[email protected]>

* Lint

Signed-off-by: phixMe <[email protected]>

---------

Signed-off-by: phixMe <[email protected]>
Co-authored-by: Willy Lulciuc <[email protected]>

* Lineage run attachment issue. (MarquezProject#2953)

Signed-off-by: phixMe <[email protected]>
Co-authored-by: Willy Lulciuc <[email protected]>

* feature: Better handling of missing environment variables in setupProxy.js file. (MarquezProject#2956)

Signed-off-by: Artur Owczarek <[email protected]>

---------

Signed-off-by: phixMe <[email protected]>
Signed-off-by: David Goss <[email protected]>
Signed-off-by: Willy Lulciuc <[email protected]>
Signed-off-by: Alper İnan <[email protected]>
Signed-off-by: Alper <[email protected]>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Artur Owczarek <[email protected]>
Co-authored-by: Peter Hicks <[email protected]>
Co-authored-by: davidjgoss <[email protected]>
Co-authored-by: Willy Lulciuc <[email protected]>
Co-authored-by: Alper İnan <[email protected]>
Co-authored-by: Willy Lulciuc <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Artur Owczarek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants