-
Notifications
You must be signed in to change notification settings - Fork 16
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
[Issue #1546] Deploy Analytics folder to AWS ECR #1566
Conversation
@@ -0,0 +1,59 @@ | |||
data "aws_iam_role" "github_actions" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from infra/api/build-repository/main.tf
, verbatim with no changes.
@@ -0,0 +1,38 @@ | |||
name: Deploy Analytics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from .github/workflows/cd-api.yml
analytics-checks: | ||
name: Run Analyics Checks | ||
uses: ./.github/workflows/ci-analytics.yml | ||
secrets: inherit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests were broken before I added this line
secrets: inherit
# This variable is slightly misnamed. It should really be called "has_migrations". | ||
# It controls whether or not the `run-database-migrations.sh` script tries to run database | ||
# migrations. The entire analytics application is going to have its schema controlled | ||
# via ETL jobs, so we don't need to run migrations in the same way as the API. | ||
output "has_database" { | ||
value = false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't intend on updating the variable name. It seems fine to just document the issue here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Fixes #1546
Time to review: 5 mins
Changes proposed
infra/analytics
cd-analytics
script that I used to confirm that the ECR repository push was workingContext for reviewers
The PR was like 95% copy pasting
Testing
https://us-east-1.console.aws.amazon.com/ecr/repositories/private/315341936575/simpler-grants-gov-analytics?region=us-east-1