Skip to content

chore: OSS -> EE mass migration #9086

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

Closed
wants to merge 3 commits into from

Conversation

eecsliu
Copy link
Contributor

@eecsliu eecsliu commented Apr 2, 2024

Description

Before merging EE and OSS, we must bring both migrations to parity. The up migration in this PR contain all the migrations that OSS is missing while remaining a no-op for EE databases. In addition, we are adding all the down migrations corresponding with the ones included in the up migration, just in case someone wants to do a down migration.

Test Plan

Make sure CI is running fine.
To manually test, remove oss-to-ee-catch-up.tx.up.sql from the migrations directory and run devcluster with a clean database. Then add the file back and restart devcluster master. This should succeed. Next, switch to EE version and run devcluster (without removing the database). The upgrade from OSS to EE should be seamless.

Commentary (optional)

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

@eecsliu eecsliu requested a review from maxrussell April 2, 2024 08:49
@eecsliu eecsliu requested a review from a team as a code owner April 2, 2024 08:49
@cla-bot cla-bot bot added the cla-signed label Apr 2, 2024
Copy link

netlify bot commented Apr 2, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit 0f8caff
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/661434674ad1040008bcf993
😎 Deploy Preview https://deploy-preview-9086--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@eecsliu eecsliu force-pushed the oss-license-check-redo-migrations branch from c1c6148 to 1af51a7 Compare April 2, 2024 18:06
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.17%. Comparing base (cf2f2be) to head (c49d7a1).
Report is 2 commits behind head on main.

❗ Current head c49d7a1 differs from pull request most recent head 0f8caff. Consider uploading reports for the commit 0f8caff to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9086   +/-   ##
=======================================
  Coverage   46.16%   46.17%           
=======================================
  Files        1174     1174           
  Lines      145186   145186           
  Branches     2413     2413           
=======================================
+ Hits        67026    67035    +9     
+ Misses      77952    77943    -9     
  Partials      208      208           
Flag Coverage Δ
backend 43.51% <ø> (+0.02%) ⬆️
harness 64.00% <ø> (ø)
web 36.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 5 files with indirect coverage changes

@eecsliu eecsliu force-pushed the oss-license-check-redo-migrations branch from 1af51a7 to 91227ca Compare April 2, 2024 21:20
@eecsliu eecsliu requested a review from a team as a code owner April 8, 2024 06:37
@eecsliu eecsliu requested a review from djanicekpach April 8, 2024 06:37
@eecsliu eecsliu force-pushed the oss-license-check-redo-migrations branch 3 times, most recently from c49d7a1 to 0f8caff Compare April 8, 2024 18:16
Copy link
Contributor

@djanicekpach djanicekpach left a comment

Choose a reason for hiding this comment

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

Seems fine from infra review perspective. This seems like the type of change where testing will be the best way to find problems and it looks like that happened.

@eecsliu eecsliu marked this pull request as draft April 17, 2024 00:20
@eecsliu eecsliu closed this May 15, 2024
@keita-determined keita-determined deleted the oss-license-check-redo-migrations branch July 16, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants