Skip to content

VDB 1627 Only manage plugin migrations#285

Merged
paytonrules merged 18 commits intostagingfrom
vdb-1627-only-manage-plugin-migrations
Sep 15, 2020
Merged

VDB 1627 Only manage plugin migrations#285
paytonrules merged 18 commits intostagingfrom
vdb-1627-only-manage-plugin-migrations

Conversation

@paytonrules
Copy link
Copy Markdown
Contributor

The transformer should only manage the plugin migrations, and is not responsible for the public schema. In addition to just pulling out the migrations:

  • The Make tasks all pass the "-table" flag when migrating the schema.
  • The docker files all use go run to ensure they are using the right version of goose
  • A copy of the schema dump from vulcanizedb is used for testing. Updating vulcanize can be done with a handy make task.
  • Compose now takes a schema flag.

Copy link
Copy Markdown
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

A few questions but in general this looks like a super awesome and elegant way to isolate the plugin schema 🥇 🚀

$(GOOSE) postgres "$(CONNECT_STRING)" up
pg_dump -O -s $(CONNECT_STRING) > db/schema.sql
$(GOOSE) -table "maker.goose_db_version" postgres "$(CONNECT_STRING)" up
pg_dump -n 'maker' -n 'api' -O -s $(CONNECT_STRING) > db/schema.sql
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a responsibility of this PR but I wonder if we should create a story to remove/rename the api schema - seems like that could get confusing with multiple plugins + every schema include in the actual API 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can add a story to Jira if you want. API definitely seems confused.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 really like the idea of updating the api schema at some point

RUN git checkout $VDB_VERSION
RUN go build
# Build migration tool
RUN go install -tags='no_mysql no_sqlite3 no_mssql no_redshift' github.com/pressly/goose/cmd/goose
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just tagging here something we discussed on the VDB PR that we need to confirm this sticks to a version of goose that supports the -table flag (and doesn't accidentally upgrade to not supporting it if they remove/rename that in a subsequent release)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and changed the docker containers to all explicitly specify their versions, just to be safe.

@paytonrules paytonrules force-pushed the vdb-1627-only-manage-plugin-migrations branch 3 times, most recently from ef3b728 to c68a3e3 Compare September 9, 2020 18:37
Copy link
Copy Markdown
Contributor

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

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

looking good, just a couple of questions 👍

$(GOOSE) postgres "$(CONNECT_STRING)" up
pg_dump -O -s $(CONNECT_STRING) > db/schema.sql
$(GOOSE) -table "maker.goose_db_version" postgres "$(CONNECT_STRING)" up
pg_dump -n 'maker' -n 'api' -O -s $(CONNECT_STRING) > db/schema.sql
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 really like the idea of updating the api schema at some point

To 2.7.0-rc5 (brand new!)
- Transformer migrations use the -table flag to store their upgrades in
the maker.goose_db_version schema.

- Explicitly create the maker schema.

- For make tests load a copy of the vulcanize schema and load it before
running tests.
This puts mcd-vdb-migrations in a different directory, and runs them
using the maker.goose_db_migrations table to track them.

- Go get goose 2.7.0-rc5 in execute dockerfile

- Add dockerbuild and execute helpers to Makefile
- Make sure vulcanize_schema only has the public schema.
- Update the integration tests to also have the public schema
- Remove getting goose from the makefile
- Update schema to latest vulcanizedb
- Update to branch version of vulcanize
- Only dump maker and api schemas, because the transformers only manage those schemas
Update the README for updating vulcanizedb
It's not used in this image.
Also ensure the  migrations come from the right locations
The tests didn't rely on them anyway.
This gets rid of a bunch of noise.
We don't do migrations in this Dockerfile.
@paytonrules paytonrules force-pushed the vdb-1627-only-manage-plugin-migrations branch from 6b58470 to fa4fbf9 Compare September 15, 2020 15:22
Copy link
Copy Markdown
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

approving since the deadline passed 🕐

@paytonrules paytonrules merged commit de4d4e4 into staging Sep 15, 2020
@paytonrules paytonrules deleted the vdb-1627-only-manage-plugin-migrations branch September 15, 2020 19:14
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.

3 participants