-
Notifications
You must be signed in to change notification settings - Fork 511
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
migrations: add ReplaceStringMigration
migration helper, simplify containerd-config-path migration, host-containers-version migration
#712
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please describe the testing you did in the description? It might be easiest to include the change to the existing host-containers-version migration in this PR so you can use that clear example.
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
6ccc58f
to
117897e
Compare
ReplaceSettingMigration
ReplaceStringMigration
migration helper, simply containerd-config-path migration
ReplaceStringMigration
migration helper, simply containerd-config-path migrationReplaceStringMigration
migration helper, simplify containerd-config-path migration
117897e
to
f2f1546
Compare
Pushes above addresses @tjkirch 's comments. Added a testing section to the PR description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please also update the host-container-version migration?
workspaces/api/migration/migrations/v0.2/migrate-containerd-config-path/src/main.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
a8e20c4
to
71299ab
Compare
ReplaceStringMigration
migration helper, simplify containerd-config-path migrationReplaceStringMigration
migration helper, simplify containerd-config-path migration, host-containers-version migration
Addresses @tjkirch 's comments. Added more testing to the PR description |
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
workspaces/api/migration/migrations/v0.2/migrate-containerd-config-path/src/main.rs
Outdated
Show resolved
Hide resolved
71299ab
to
5573b59
Compare
Addresses @tjkirch 's comments. |
workspaces/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
Adds new common migration `ReplaceStringMigration` for replacing the string value of some specified setting in the datastore.
Simplifies containerd config path migration with new `ReplaceStringMigration` migration helper
Use new `ReplaceStringMigration` helper to simplify host-containers-version migration
5573b59
to
717cefd
Compare
Addresses @tjkirch 's comment. Tested migrations and they still work as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚦
Issue #, if available: N/A
Description of changes:
Adds new common migration helper
ReplaceStringMigration
for replacing the string value of some specified setting in the datastore.Simplifies
migrate-contrainerd-config-path
withReplaceStringMigration
.Testing done:
Locally testing updated
migrate-containerd-config-path
migration, backward migration:Forward migration:
Locally testing updated
migrate-host-containers-version
migration, backward migration:Forward migration:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.