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

Insert temp tables before other migrations take place. #3119

Merged
merged 1 commit into from Jun 23, 2022

Conversation

ChrisPenner
Copy link
Contributor

@ChrisPenner ChrisPenner commented Jun 15, 2022

Old Context:

  • Arya is looking into how to 'freeze code' which is used in migrations because a recent change to saveObject; Currently looking into copy-pasting all existing code and 'Weeder'ing out any code not used by the migration
    • Chris suggests that maybe we should look into simpler options to save time while we're in crunch mode.
    • Option 1: Use an old version of UCM to do some of the updates
      • Arya: kind of janky
    • Option 1b: separate migration tool
    • Option 2: Copy monorepo to migration-specific package + weed out unnecessary dependencies -A
      • getting weeder working might be a big job / low priority?
    • Option 3: Copy monorepo to migration-specific package + don't weed out unnecessary dependencies
      • high compilation times
        • no, this should be okay if pulled out of the monorepo
      • high binary sizes
    • Option 4: Copy monorepo to migration-specific package + weed out some unnecessary dependencies -M/T
      • Just try the insert and check for a 'missing table exception' or w/e
    • Option 5: Check for Temp Table existence (or schema version) in saveObject
      • global variables -A -C (temporarily, just until we have more time for a better solution)
    • Option 6: Check/save schema version in runTransaction or something
    • Note: We want a principled way to handle this in future migrations, regardless.
    • Note: Majority of codebases don't need to be migrated
    • Note: Migrations don't need to be run in the same transaction
    • Note: In some cases, pushing a codebase to Share w/ old ucm and pulling it back with a new one might be faster / more convenient.

New:

This PR take an alternative approach, the only requirement for old migrations to work is that these tables EXIST, they don't need to be populated with anything at all, so we can just add those tables as a 'pre-migration' step whenever we run a migration.

It's still a hack that should be removed later, but it's safer, simpler, adds less junk code, and is more performant than our other approaches.

  • Tested on commit 5cf1da4 from base which is a v1 codebase.

@ChrisPenner ChrisPenner changed the base branch from trunk to travis/update-indices June 15, 2022 22:35
@ChrisPenner ChrisPenner marked this pull request as ready for review June 15, 2022 22:46
@tstat tstat force-pushed the travis/update-indices branch 2 times, most recently from 19e78cb to 535eaf9 Compare June 16, 2022 18:29
This is a bit of a hack to handle the fact that saveObject now tries to
flush temp entity tables, and they might not exist on old schema
versions
@ChrisPenner ChrisPenner force-pushed the cp/fix-old-migrations-table-creation branch from e75a954 to 901d838 Compare June 17, 2022 14:48
@ChrisPenner ChrisPenner changed the base branch from travis/update-indices to arya/ooo-sync June 17, 2022 14:49
@ChrisPenner ChrisPenner requested review from mitchellwrosen, aryairani and tstat and removed request for mitchellwrosen June 17, 2022 14:50
Base automatically changed from arya/ooo-sync to trunk June 17, 2022 20:11
@aryairani aryairani merged commit 1ccf6bf into trunk Jun 23, 2022
@aryairani aryairani deleted the cp/fix-old-migrations-table-creation branch June 23, 2022 15:50
@mitchellwrosen
Copy link
Member

Note: let's change the create table if not existss back to create tables once we implement the freezy fix.

@aryairani aryairani mentioned this pull request Jun 23, 2022
12 tasks
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.

None yet

3 participants