Skip to content

Vdb 1294 update docker compose config and directions#192

Merged
paytonrules merged 4 commits intostagingfrom
vdb-1294-update-docker-compose-config-and-directions
May 29, 2020
Merged

Vdb 1294 update docker compose config and directions#192
paytonrules merged 4 commits intostagingfrom
vdb-1294-update-docker-compose-config-and-directions

Conversation

@paytonrules
Copy link
Copy Markdown
Contributor

Extract run-migrations from startup-script, and then reference the docker-compose setup in the directions first.

This PR has a bit of a chicken-and-egg problem as the Docker image needs to be updated before the directions are correct. I validated this using a local image, but that's not a perfect setup.

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.

Looking good! Happy to approve as long as we can verify the existing CD pipeline will continue working as expected with these changes

The database needs to be initialized once before bringing up the system. That can be by running, from the root directory,

```bash
docker-compose -f dockerfiles/docker-compose.yml run execute ./run_migrations.sh
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 to double check - have you run this locally and verified the migrations run before the containers run their own migrations? Asking because I've seen the migrations take a long time (~3 minutes) before

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.

Just double-checked and when that command is run locally it only starts the postgres image, and the execute image. The others aren't run, and the execute image actually shuts down when run_migration is done, because it overrides the startup command.


### Docker Setup

This repository provides a docker-compose setup to manage the multiple container-based deployment for you, if at all possible you should use that setup.
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.

This seems like the right approach, but maybe worth mentioning the need to be sure the resources allocated to Docker are substantial - I think we'll hit lower limits really fast with the ethereum node included

contract data via [event](./transformers/events) and [storage](./transformers/storage) transformers. The VulcanizeDB
repository includes a [general description](https://github.com/makerdao/vulcanizedb/blob/staging/documentation/custom-transformers.md)
about transformers.
1. `execute` uses the raw Ethereum data that has been synced into Postgres and applies transformations to configured MCD contract data via [event](./transformers/events) and [storage](./transformers/storage) transformers. The VulcanizeDB repository includes a [general description](https://github.com/makerdao/vulcanizedb/blob/staging/documentation/custom-transformers.md) about transformers.
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.

Maybe doesn't need to be part of this PR, but you now also need to run extractDiffs to get the storage diffs used by the storage transformers

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.

That makes a lot of sense - probably a separate PR?

1. Create a user for yourself that is able run migrations and add extensions.
1. `createdb vulcanize_public`
1. Migrating the database manually is unnecessary as the commands handle this.
1. Migrate the database using the `make migrate` task in this repository.
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.

fwiw I believe execute and composeAndExecute do handle running the migrations for you, but if you went that path you would need to be sure to run those commands first since the vulcanizedb migrations alone will moot out migrations in this repo.

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.

You're correct - I changed the directions for this reason. It might be good to re-warn the reader here of what will happen if you do things in the wrong order.

echo "Could not run migrations. Are the database details correct?"
exit 1
fi
source run_migrations.sh
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.

So this will still run the migrations automatically, right? Just double checking we don't need to modify the CD pipeline with these changes

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.

It did locally. Is there a good CD pipeline test you can think of?

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 was able to build the execute Docker container and run execute, and sure enough it ran the migrations.

This allows running the migrations as a separate step, and a one-off
before running docker-compose up.
Running it this way should just work.
@paytonrules paytonrules force-pushed the vdb-1294-update-docker-compose-config-and-directions branch from d56d8e8 to 180bfd5 Compare May 27, 2020 21:11
@paytonrules paytonrules requested a review from rmulhol May 27, 2020 21:53
@paytonrules paytonrules merged commit ea81ccc into staging May 29, 2020
@paytonrules paytonrules deleted the vdb-1294-update-docker-compose-config-and-directions branch August 26, 2020 15:52
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