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

[ETL-395] Modify prod stack configs to use new pre-ETL bucket name #31

Merged
merged 4 commits into from
Mar 30, 2023

Conversation

rxu17
Copy link
Contributor

@rxu17 rxu17 commented Mar 30, 2023

Purpose: We recently created the new pre-ETL (input data) in the prod account, and now the prod stack configs need to be adjusted to use the new bucket. Here a stack group config was also created to house the prod specific bucket names for ease of use across all the prod stacks.

@rxu17 rxu17 requested a review from a team as a code owner March 30, 2023 17:56
input_bucket_name: recover-input-data
cloudformation_artifact_bucket_name: recover-cloudformation
intermediate_bucket_name: recover-intermediate-data
processed_data_bucket_name: recover-processed-data
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of including the bucket names in the stack group config we should list the bucket stacks as dependencies in the relevant config files and reference the bucket name using

!stack_output_external bucket-stack::BucketName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on our recent slack discussion, and to avoid redundancy in our stack dependencies with the buckets, we will use the stack_group_config way

Copy link
Contributor

Choose a reason for hiding this comment

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

You'll also want to use a stack_group_config reference for the BucketName parameter in the bucket stacks themselves: config/prod/s3-cloudformation-bucket.yaml, config/prod/s3-ingestion.yaml, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to do the same in dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I have a ticket that encompasses all of that here: https://sagebionetworks.jira.com/browse/ETL-401

@rxu17 rxu17 requested a review from philerooski March 30, 2023 19:11
@rxu17 rxu17 merged commit 19f2132 into main Mar 30, 2023
@rxu17 rxu17 deleted the etl-395 branch March 30, 2023 22:28
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