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

feat(backend): User data migration #1967

Merged
merged 143 commits into from
Aug 21, 2024
Merged

feat(backend): User data migration #1967

merged 143 commits into from
Aug 21, 2024

Conversation

bitdivine
Copy link
Member

@bitdivine bitdivine commented Aug 5, 2024

Motivation

We would like to migrate user data to a new canister. The groundwork for the migration code is already in place. Now we need the actual migration code.

Changes

  • Populate the skeleton code for migrating data.
  • Populate the skeleton e2e test.

Tests

A pocket-ic test is included that exercises the migration from end to end. The main difference from production code is that the pocket-ic code steps explicitly whereas normally the migration is driven by a timer.

@bitdivine bitdivine marked this pull request as ready for review August 20, 2024 18:38
@bitdivine bitdivine requested a review from a team as a code owner August 20, 2024 18:38
@bitdivine bitdivine changed the title feat(backend): Data upload download apis feat(backend): User data migration Aug 20, 2024
Copy link

@frederikrothenberger frederikrothenberger left a comment

Choose a reason for hiding this comment

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

LGTM!

pub async fn step_migration() {
/// The next chunk of user tokens to be migrated.
fn next_user_token_chunk(last_user_token: Option<Principal>) -> Vec<(Principal, Vec<UserToken>)> {
let chunk_size = 5;

Choose a reason for hiding this comment

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

This is a very small chunk size. Will this be increase / made configurable in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oisy doesn't have many users. In future I expect this code to be gone, at which point all the migration code that exists will be infinitely configurable. 😉

///
/// # Errors
/// - Throws a `MigrationError::DataMigrationFailed` if the data transfer fails.
macro_rules! migrate {

Choose a reason for hiding this comment

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

Interesting. I would have approached that using generics, but this is probably cleaner. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

I think generics are easier to debug than macros and more familiar to most people. So I tend to avoid macros where possible. But I am not sure it is possible to pass say $chunk_variant in a generic function, so I think this is the only option. I didn't spend a long time deciding though; I could be wrong.

@bitdivine bitdivine merged commit ef5bec7 into main Aug 21, 2024
18 checks passed
@bitdivine bitdivine deleted the data-upload-download-apis branch August 21, 2024 08:35
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.

2 participants