[#12048] Add migration script for Usage Statistics #12798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin9foong
requested review from
FergusMok,
NicolasCwy,
domlimm and
ziqing26
kevin9foong
changed the title
kevin9foong
added
s.ToReview
ziqing26
requested changes
Feb 24, 2024
Comment on lines
+35
to
+38
| @Override | ||
| protected boolean isMigrationNeeded(teammates.storage.entity.UsageStatistics entity) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Would it be possible to add a check: only returns true if the entity has not migrated yet? The other script does not have that check yet (only return true) as well
There was a problem hiding this comment.
Sure, the check will search the cloudSQL DB and check if another record with the same Id alr exists
There was a problem hiding this comment.
Hi @ziqing26, shall we figure out how to do this check as a team tomorrow? since it likely involves including some kind of HibernateUtil to check if the SQL side already has an entity with the same ID.
We could implement the Hibernate querying logic across the board in the main base script.
There was a problem hiding this comment.
Yep using HibernateUtil to check should be the idea.
ziqing26
added a commit
that referenced
this pull request
Mar 10, 2024
* Add seed script * [#12048] Add DataMigrationEntitiesBaseScriptSql and DataMigrationForAccountSql (#12766) * Add hibernate connection * Add DataMigrationEntitiesBaseScriptSql and example Account Migration * [#12048] Add verify to seed db (#12767) * Add verify * Revert secrets * Clean up seed script (#12768) * Fix account migration script * Add client property * Add SQL notification migration script * Modify seed script to persist to local database store * Data migration for account * Add seed data for data migration * Add Account and Read Notification * Remove comments * [#12048] Add migration script for Usage Statistics (#12798) * Add migration script for UsageStatistics * Add seed script for usage statistics and add ofy support * Set preview to true by default * [#12048] Add migration script for Account Request (#12799) * Add migration script for acc req * Update migration script * Add checks to notification script (#12836) * Add checks to notification script * Fix comments * Create script to verify row count for non-course entities (#12824) * Create script to verify row count * Add read notification verification * Add comment * Add base script for verifying migrated attributes (#12841) * Add datastore entity comparison function (except readNotification) * Add verify attribute entity base script Co-authored-by: Kevin Foong <[email protected]> * Add verify usage statistics * Add verification script for account request * Amend base script to fetch lazily loaded keys * Fix migration verification for notification * Add migration verification script for account * Save progress * Add changes * Fix bug * Add changes * Add support for pagination * Add changes (#12846) * Add a script to remove datastore non course entities and fix progress bar in seeddb * Add warning for the script * Add user check for removal script * Revert remove script * fix seed script to populate notification UUID correctly * Remove unnecessary whitespace * Add connection verification script * Remove only one entity in Verify Connection Script * Fix seeding of data for data migration (#12873) * Fix notification and readNotification from having different endtime * Fix seed data to create notification to nearest millisecond * Remove commented out println * Uncomment databundle persistance * Remove unnecessary comments and fix format * Revert logs in EntitesDb * V9 Migration: Fix verification pagination, improve logging (#12874) * Fix numPages division bug, add logging, testing paging * Add logging * Remove unnecessary function * Remove unnecessary files, shift some functions * Clean up branch * Fix lint errors * Fix lint errors v2 * Fix lint errors v3 * Fix spotBugsTest --------- Co-authored-by: Nicolas Chang <[email protected]> Co-authored-by: FergusMok <[email protected]> Co-authored-by: Kevin Foong <[email protected]> Co-authored-by: Kevin Foong <[email protected]> Co-authored-by: Kevin Foong <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Write script to migrate all usage statistics from datastore to cloudSQL
Verification:
Seed DB with UsageStatistics.
./gradlew -PuserScript=sql/SeedUsageStatistics execScriptRun the migration script in preview (im using non preview to check on live CloudSQL in the screenshot)
./gradlew -PuserScript=sql/DataMigrationForUsageStatisticsSql execScriptWe can see the 2 seeded UsageStatistics are reported to be migrated.
Verify on CloudSQL
the fields seem to match seeded datastore records.