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

Method for updating data on version upgrade #247

Closed
aaronjudd opened this issue Jan 19, 2015 · 10 comments
Closed

Method for updating data on version upgrade #247

aaronjudd opened this issue Jan 19, 2015 · 10 comments
Assignees
Milestone

Comments

@aaronjudd
Copy link
Contributor

aaronjudd commented Jan 19, 2015

Add some kind of trigger when a package version changes to allow programmatic migration of existing data to any new updates.

ie: Update from v0.2.1 -> v0.2.2
Trigger: Move shipping object out of Settings collection and adapt, insert into Shipping collection

@bluerabbbit
Copy link

+1

@aaronjudd
Copy link
Contributor Author

@jshimko we should review this requirement as part of the plugin/module architecture.

@aaronjudd aaronjudd added the core label Sep 9, 2016
@aaronjudd aaronjudd modified the milestone: Core Architecture Sep 9, 2016
@aaronjudd aaronjudd added ready and removed backlog labels Sep 16, 2016
@aaronjudd aaronjudd added this to the v0.19.0 milestone Sep 16, 2016
@aaronjudd
Copy link
Contributor Author

aaronjudd commented Oct 14, 2016

@zenweasel I believe this is the issue where you would want to discuss migration implementation details. There's also some background discussions worth reading in the import issue #467

@zenweasel
Copy link
Collaborator

I have come into a scenario where I feel like I need to update data on version upgrade. Looking back it's sort of needless but oh well.

In the last version we built versions of the OrderSearch and AccountSearch index collections even though they weren't used for anything. But now in this version with actual Order and Account search the collections are different and the search will error out if the old collections are in place. (tested and verified)

So I added, just as something to look at a very simple implementation of migrations using the percolate:migrations package. You can see it here https://github.com/reactioncommerce/reaction/tree/f0e037b550d651f4826c188632383a35f3a2441f/imports/plugins/included/migrations

I'm fine if we want to throw this away, just wanted to try something and see how it actually worked in practice.

@aaronjudd
Copy link
Contributor Author

I know that this probably requires a more in depth discussion, but I also think we don't need to over engineer this, as long as we leave ourselves some easy upgrade opportunities.

I'm sure there's far more robust solutions, but this gets the job done immediately in #1494. Not sure if this is correct, but I tend to associate migrations with data, and versions with update or upgrades of software, so I also prefer your "versions" naming better.

Looks like we can configure collectionName: "migrations" as collectionName: "versions" to mirror your convention of versions elsewhere. If you renamed the folder "migrations", "updates" could be used instead of "versions" for the actual imports and that would make sense.

I'd suggest that it'd be ok to move from included to core, I can't see us wanting this to be optional.

import { Migrations as Versions} could default export, so that you can just include and export

import { Versions } from "imports/plugins/core/versions"

I think that could make it a lot easier to reuse in other plugins without things breaking if we replace (and that's pretty likely). I'm assuming that you can have multiple instances of Migrations.add?

Can this support our versioning from the package.json? (ie, 0.17.0)? That way we'd be using a consistent reference.

Maybe we could use the version direct from reaction -v, ie: have the reaction-cli export checkVersions() whenever it's run so that we could then use as global application version. (and maybe update an entry in the registry, for other use cases). I'm not sure how we'd handle that for production builds, meteor cli.. or if we're exporting this somewhere/somehow that I've forgotten.

The actual version of the app's running package.json is 0.0.0 because it's a fake package that the Meteor build creates, so the env export stuff is a bit of a work around, but it'd also be useful functionality as we start moving plugins to npm packages in the future, as we'll be able to use the version from their manifests in the registry as well.

What happens on failure? Is it graceful?

I'd be more comfortable if this was adapted to npm (as in their "Call for Maintainers" issue), as I think architecturally we should be moving as quickly as possible away from any Meteor packages. Maybe something we need to shepherd if need be?

@ghost ghost assigned aaronjudd Oct 15, 2016
@ghost ghost added review and removed ready labels Oct 15, 2016
@zenweasel zenweasel self-assigned this Oct 15, 2016
@zenweasel
Copy link
Collaborator

One issue that I thought of is that when I do a PR that may have a migration in it I may not know which "version" that code may be in. Let's say it's a bug fix and we may issue a point release or we might wait until the next release depending on how severe, etc.

This is probably a process issue rather than a technical one but it would be one argument against not tethering the migration version to the software version (at least not in the way that I have it here)

Maybe it's just an issue where the release lead just needs to cleanup/align all the migrations with the release to prepare it for merging.

@zenweasel
Copy link
Collaborator

To answer your question about errors, I tested this this morning (totally on purpose of course). If a migration throws an exception that migration halts and that migration record is marked "locked" and migrations will not move forward until that migration is unlocked (which is done by just manually editing the record). So depending on where it failed you could mark that migration unlocked (so it runs again) or completed.

@aaronjudd
Copy link
Contributor Author

@zenweasel to capture our chat, the plan is changed a bit.. to use migrations as the namespacing. I think we then also should use, Migrations (capitalized, as the collection name). Since the migration packages isn't really version based, we'll probably want to relate this to versioning in the future, but the migration.version itself should the "version of the migration", not the application package version. Thus, ordered integers starting with 0 (un-migrated), 1 , 2, 3 ,etc.

@zenweasel
Copy link
Collaborator

@aaronjudd Should we close this issue and just deal with specific issues/enhancements as they come up.

@zenweasel
Copy link
Collaborator

Closed via #1494. Simple version using percolate:migrations

@ghost ghost removed the review label Oct 19, 2016
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

No branches or pull requests

4 participants