Add move checks #560
Add move checks #560
Conversation
| }) | ||
|
|
||
| // Create checkpoint table for remaining tests | ||
| _, err = sourceDB.ExecContext(t.Context(), `CREATE TABLE resume_src._spirit_checkpoint ( |
There was a problem hiding this comment.
Is it potentially annoying for future you to not create the checkpoint with the actual checkpoint code?
There was a problem hiding this comment.
There will be a cyclical dependency because that would require runner.go:createCheckpointTable() to be called, which needs to import the move package.
If I extracted createCheckpointTable() to its own package for checkpoints that would make this possible - but there is not too much benefit to that right now.
| err := r.SourceDB.QueryRowContext(ctx, | ||
| "SELECT 1 FROM information_schema.TABLES WHERE table_schema = ? AND table_name = ?", | ||
| r.SourceConfig.DBName, checkpointTableName).Scan(&checkpointExists) | ||
| if err == sql.ErrNoRows { |
There was a problem hiding this comment.
Does anything in the checkpoint table itself need to be checked at this stage?
There was a problem hiding this comment.
I could, but restoring the checkpoint is also able to return an error in pkg/move/runner.go:resumeFromCheckpoint()
My preference is not to because if I change the checkpoint format (it happens) the code is non-dry. This is because the check should only check and is not expected to load or mutate structures.
If I move checkpoints to a separate package, that would be a different argument though; they could call the same helper.
2 participants
A Pull Request should be associated with an Issue.
Fixes #534
The checks system in migrations moves some of the non-mutating inspections of configuration, state, options into a separate package. Because it depends on
Resourceswhich are specific to migrations, it is hard to repurpose this to moves. So I've moved it to pkg/migration/check/ and created a new pkg/move/check with similar semantics.Full list of changes: