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

✨ Migration runner - first iteration #7501

Merged
merged 1 commit into from Oct 10, 2016

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Oct 6, 2016

refs #7489

This is a pure implementation of the new migration runner.
The connection to Ghost happens in the PR, which comes in a few seconds.

The idea:
Sephiroth (we can rename if we want) has a command line and JS API.
It runs a type (seed, init, migrate) in one single transaction.
It takes over the responsibility to add each migration file into a collection called migrations.

Each type (seed, init, migrate) will work basically the same. It takes all migration scripts in the type folder and executes all.

By storing each script into the database, we can easily check the health of the database.
preTask will check if the migration script was already running.
postTask is triggered when a script finished and stores the script name into the migrations collection.

We still need to ensure that a migration script itself can run twice if it get's executed twice.
So we have two layers of protection. I already took care of for example: owner creation.

isDatabaseOK is a check to ensure seed and init happened. This function is right now not optimised. It just checks if the type init was already executed. Need to optimise that behaviour later when making final changes.

node core/server/data/sephiroth/bin/sephiroth --help

  Usage: sephiroth [options] [command]


  Commands:

    init [options]   populate tables

  Options:

    -h, --help  output usage information

If you remove your sqlite db and run:

node core/server/data/sephiroth/bin/sephiroth init

You can also run it twice, to see that it will skip the init scripts.

This won't work with Ghost yet!Therefor we have the next PR #7502.
So you can just use the runner and then delete your db again.

It uses logging and config, which is a dirty require, but i think that is super OK for now. Everything else will act mostly standalone.

@kirrg001 kirrg001 changed the title [WIP] ✨ Migration runner - first iteration ✨ Migration runner - first iteration Oct 6, 2016
refs TryGhost#7489
- add independent migratio runner
- add init script
- this is not connected to Ghost yet, but next PR will
@ErisDS ErisDS modified the milestone: 1.0.0-alpha.4 Oct 7, 2016
@ErisDS ErisDS self-assigned this Oct 7, 2016
@ErisDS
Copy link
Member

ErisDS commented Oct 10, 2016

Sephiroth (we can rename if we want) has a command line and JS API.

😳
What is one of those?

To clarify here, as I understand:

  • init = initial database migrations, calculated from schema.js
  • migrate = make an incremental change between versions.
  • seed = ? Is this initial, or migration? Do we need one or both?

@ErisDS
Copy link
Member

ErisDS commented Oct 10, 2016

Maybe it's a good idea to create the folder structure you intend for the other parts & use .gitkeep files to keep the structure until we need it?

@kirrg001
Copy link
Contributor Author

seed = ? Is this initial, or migration? Do we need one or both?

Hmm i didn't really really start with seeds yet, but here is what i thought how it will work:
Init and seed are two different commands. So when having different types (seed, init), it makes sense to require different folders. I want to keep flexibility. Maybe we will have a handful of seed scripts later. I am not 100% sure how much of the fixture logic we will keep.

  • init
    • 1-tables
  • seed
    • 1-owner
    • 2-welcome post
  • 1.0

@kirrg001
Copy link
Contributor Author

Maybe it's a good idea to create the folder structure you intend for the other parts & use .gitkeep files to keep the structure until we need it?

Hmmm I think it's ok to extend the migration runner in the next iteration for seeds and migrations.

Facts for next iterations:

  • sephiroth logic can change in the next iterations to make code easier and re-use more logic of db init logic
  • there will be a decision for final folder structure sephiroth expects
  • there will be a decision for the old fixture logic
  • migrate is straight forward i think

@ErisDS
Copy link
Member

ErisDS commented Oct 10, 2016

Ok - one thing that's worth flagging up here, I think:

The biggest, longest-running pain point that seriously impacted our ability to ship, was being able to safely migrate permissions fixtures. Whilst the code we have for doing that is undeniably large and complex, it did create a very easy way for us to add to the permissions fixtures, which has allowed us to ship more in the last few months where previously we were stuck.

I think it's important to consider that initialising fixtures, and changing or adding to them are two separate things which we will also definitely have use cases for, and that doing it needs to be super simple. Also perhaps worth thinking about the fact that tests also need fixtures (Sometimes we want to mirror the defaults, sometimes you want custom fixtures).

There is a subtle difference here in terminology that I think is making me feel uncomfortable. The concept of "seeds" and "seeding" have been, everywhere I've encountered them (rails, bookshelf), a 1-time thing. Rather than something that can easily be adapted or migrated.

@kirrg001
Copy link
Contributor Author

There is a subtle difference here in terminology that I think is making me feel uncomfortable. The concept of "seeds" and "seeding" have been, everywhere I've encountered them (rails, bookshelf), a 1-time thing. Rather than something that can easily be adapted or migrated.

Seeds are a one time thing. When your database has reached the "was seeded" state, you can not seed it again. You can try, but it will skip, because each script which was running, is remembered in the migrations collection.

  • seeds
    • 1-owner
    • 2-welcome-post

Does this confuses you?This structure was created in my head because seeds are basically just a migration script. And it makes things easier to read when having not everything in one big file like core/server/migrations/seeds.js.

The biggest, longest-running pain point that seriously impacted our ability to ship, was being able to safely migrate permissions fixtures. Whilst the code we have for doing that is undeniably large and complex, it did create a very easy way for us to add to the permissions fixtures, which has allowed us to ship more in the last few months where previously we were stuck.

I think it's important to consider that initialising fixtures, and changing or adding to them are two separate things which we will also definitely have use cases for, and that doing it needs to be super simple.

Initialising fixtures is seeding the database. Changing/adding them is a migration script.

Also perhaps worth thinking about the fact that tests also need fixtures (Sometimes we want to mirror the defaults, sometimes you want custom fixtures).

Ja sure!

I am a bit confused about your comment, maybe we can talk in DM shortly, not sure what you are going to point out here 👍

@ErisDS
Copy link
Member

ErisDS commented Oct 10, 2016

👍 let's chat, nothing serious, just historic info / knowledge to share.

@ErisDS ErisDS merged commit c4fa342 into TryGhost:master Oct 10, 2016
mixonic pushed a commit to mixonic/Ghost that referenced this pull request Oct 28, 2016
refs TryGhost#7489
- add independent migratio runner
- add init script
- this is not connected to Ghost yet, but next PR will
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
refs TryGhost#7489
- add independent migratio runner
- add init script
- this is not connected to Ghost yet, but next PR will
@ErisDS ErisDS removed their assignment Jun 22, 2021
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.

None yet

2 participants