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

MB-1421 - new migrations layout #3496

Merged
merged 21 commits into from Feb 11, 2020
Merged

MB-1421 - new migrations layout #3496

merged 21 commits into from Feb 11, 2020

Conversation

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj commented Feb 7, 2020

Description

To support Orders as its own service a new set of migration files will need to be set up. This PR attempts to put the migrations into namespaced folders.

Setup

make db_dev_reset db_dev_migrate

Code Review Verification Steps

  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • Request review from a member of a different team.
  • Have the Jira acceptance criteria been met for this change?
@chrisgilmerproj chrisgilmerproj self-assigned this Feb 7, 2020
…n building, ie with ldflags
This reverts commit ca08ecf.
@chrisgilmerproj chrisgilmerproj marked this pull request as ready for review Feb 10, 2020
@chrisgilmerproj chrisgilmerproj requested a review from transcom/truss-infra Feb 10, 2020
Dockerfile.local Show resolved Hide resolved
Dockerfile.migrations_local Show resolved Hide resolved
Copy link
Contributor

tinyels left a comment

The concept of "local secure migration" is unclear to me. Is this something we are introducing here, or is it just a new phrase for the placeholder stubs we would use before? I worry that calling them secure will lead engineers to think they should be committing migrations with real data in them.

@@ -154,7 +154,7 @@ We are piggy-backing on the migration system for importing static datasets. This
2. Edit the production migration first, and put whatever sensitive data in it that you need to.
3. Copy the production migration into the local test migration.
4. Scrub the test migration of sensitive data, but use it to test the gist of the production migration operation.
5. Test the local migration by running `make db_dev_migrate`. You should see it run your local migration.
5. Test the local secure migration by running `make db_dev_migrate`. You should see it run your local secure migration.

This comment has been minimized.

Copy link
@tinyels

tinyels Feb 11, 2020

Contributor

what is meant by "local secure migration" here? How is it different than before?

@chrisgilmerproj

This comment has been minimized.

Copy link
Contributor Author

chrisgilmerproj commented Feb 11, 2020

The concept of "local secure migration" is unclear to me. Is this something we are introducing here, or is it just a new phrase for the placeholder stubs we would use before? I worry that calling them secure will lead engineers to think they should be committing migrations with real data in them.

The nomenclature we're using is really confusing folks. To say "put a local migration here but a secure migration somewhere else" seems to give folks, especially new folks, the wrong impression. What they are actually doing is putting in a stub for a secure migration locally and a secure migration in S3. So a "local secure migration" is more accurate than not acknowledging the local migration is in fact tied to a secure migration. You can't have one without the other so hopefully this fixes things.

Copy link
Contributor

tinyels left a comment

Approved, with provision that there is a post in slack #announcements about the changes so team is aware of new locations for things and new terminology.

@gidjin
gidjin approved these changes Feb 11, 2020
Copy link
Contributor

gidjin left a comment

🚀 with Erin's provision

@chrisgilmerproj chrisgilmerproj merged commit 97b6d7f into master Feb 11, 2020
15 checks passed
15 checks passed
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_storybook_app Your tests passed on CircleCI!
Details
ci/circleci: build_tasks Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: check_generated_code Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
@chrisgilmerproj chrisgilmerproj deleted the cg_new_migrations_layout branch Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.