feat: add process to track and run data migrations #382
Conversation
| pub async fn needs_migration(&self) -> Result<bool> { | ||
| let new_install = self.is_new_install().await?; | ||
|
|
||
| if new_install { |
There was a problem hiding this comment.
nitpick: this feels like a weird place for this logic to live. Seems like there should be an init method or something like that that is called once at startup that handles this, rather than bundling logic into this boolean accessor method.
That said this migration code is going to be fairly self-contained and mostly only run during startup anyway, so it probably doesn't matter that much.
There was a problem hiding this comment.
Yeah, agreed. I changed it to happen at start up and refactored some things into functions.. sorry, I started with a one line change and then couldn't help myself.
Thinking about this made me realize that this should all be behind &mut and that ceramic one should probably expose a CeramicDataMigration service or something so the binary is completely unaware of all store crate things. I started making those changes and backed them out because that's something for another PR.
…hile running - looks like more than it is. pulled some queries into the version struct and adjusted signatures accordingly (&self/&mut self/Self, removed async) - made things more retry friendly. fixed previous version query to exclude the current version and not error if inserting migration as completed a second time
dav1do
force-pushed
the
feat/aes-130-data-migrations
branch
from
83a3068
to
e46e101
Compare
|
So I realized the migrations to extract metadata isn't as urgent as I thought (I fixed IOD to not have bugs without it, and it might be more efficient tbh - will have the cleaned up PR today, still need to split it/make it easier to review). I do think we want to do it before GA, but I don't think I have the new table structures schema quite right, so it might make sense to pause this for a couple days. This doesn't really do much, and it doesn't hurt to merge this and store version info, but we may want to track Anyway, @stbrody, curious to hear your thoughts/discuss briefly before you spend any more time on this (and I really appreciate how much you've already spent). |
The upcoming changes to normalize some of event payload requires running a data migration a la nosql style. This provides tables to track the daemon version and the state of migrations. They can be run by calling the daemon with the
--migrate-dataflag. If there are required migrations, the daemon will fail to start.