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

Remove Flyway and move migrations to Sequelize #97

Merged
merged 19 commits into from
May 20, 2020
Merged

Conversation

rmeritz
Copy link
Contributor

@rmeritz rmeritz commented May 19, 2020

  • Remove the flyway migration and replace with sequelize migrations and sequelize seeds
  • I tried several methodologies to create the initial migration. I tried using a command-line tool but it was no longer maintained. I tried writing it by hand with the generated models as my starting point, but there were very subtle differences in the index creation that caused errors. I ended up going with using a pgdump of the original state of the database and running the commands from that because it creates a db in the exact same state as before. This means the first migrations is non-traditional but it still allows us to use normal sequelize migrations moving forward which was the main point of this work.
  • Setup new migrations to run on both Vagrant and locally
  • Change all json datatypes to a jsonb
  • Update the local Vagrant configuration to use a separate github oauth app so the sign up flow works correctly when testing with Vagrant
  • Update documentation of how to work with the db locally
  • Install the dotfile-cli as a way to easily load the environment variables
  • Add configuration for the sequelize-cli so it can be used with yarn run sequelize -- <ARGS>
  • Update documentation for information on how to deploy and how to test deploys with Vagrant
  • I've tested the app locally and in Vagrant and deleted the db on staging and deployed there

rmeritz added 15 commits May 19, 2020 13:58
Without passing in additional configuration options.
You can use dotenv to load the enviornment variable and it is much
cleaner than exporting than manually.
I looked into a bunch of ways of doing this automatically but none of
them worked with the current version of sequelize but it is the most
asked for plus-oned feature there is.

I just ended up writting it up manually.
Additionally remove flyway migraions and add deploy documentation.
This has the correct redirects.
Now `yarn run sequelize -- db:migrate:undo` works.
@rmeritz rmeritz requested review from spectranaut and evmiguel May 19, 2020 22:08
evmiguel added 3 commits May 20, 2020 15:03
resolving merge conflict due to capitalization file name change (Database.md -> database.md)
Copy link
Contributor

@evmiguel evmiguel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmeritz Looks good. I added some more to the documentation for changing a flyway managed db to sequelize management. Nonblocking question: what is server/migrations/.gitignore for?

@@ -13,6 +13,8 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
# conflict with that of another project.
config.vm.network :private_network, ip: '192.168.10.40'

config.vm.post_up_message = 'The app is now running at http://192.168.10.40/'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vagrant changes with config update work beautifully!

yarn run dotenv -e config/dev.env psql
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

const Promise = require('bluebird');
const fs = require('fs');

module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, and I think it's ok to create our initial state from the flyway dump.

@@ -0,0 +1,15 @@
'use strict';

module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -0,0 +1,15 @@
'use strict';

module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -0,0 +1,15 @@
'use strict';

module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving these seeders!

@rmeritz
Copy link
Contributor Author

rmeritz commented May 20, 2020

I added some more to the documentation for changing a flyway managed db to sequelize management.

Your docs look really good. Thanks!

Nonblocking question: what is server/migrations/.gitignore for?

It can be removed. It was added so that the folder would be tracked by git even though there was nothing it in. But then I put stuff in the folder and it became irrelevant. It had made since at some point though.

@evmiguel evmiguel merged commit bb3aeb0 into main May 20, 2020
@evmiguel evmiguel deleted the migrations-rmeritz branch May 20, 2020 20:51
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.

2 participants