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

Check in role manager dependencies #71

Merged
merged 5 commits into from
Jul 15, 2024
Merged

Check in role manager dependencies #71

merged 5 commits into from
Jul 15, 2024

Conversation

lorenyu
Copy link
Collaborator

@lorenyu lorenyu commented Dec 5, 2023

Ticket

n/a

Changes

  • Check in role manager python dependencies
  • Remove local exec pip3 install step for database module

Context for reviewers

A number of people were confused by the fact that the terraform plan is never clean due to the triggers_replace = timestamp(). There were also concerns about the dependency on pip3. This change adds the role manager dependency to the source code, which eliminates the need for the local-exec provisioner step and also removes the dependency on pip3. The cost is that there is now some vendor code in the /modules/database/role_manager/vendor folder.

Testing

Ran make infra-update-app-database
image

Ran it again to show that terraform plans now will show up as clean without the force trigger to replace
image

Ran make infra-update-app-database-roles
image

Ran make infra-check-app-database-roles
image

Preview environment

♻️ Environment destroyed ♻️

@lorenyu lorenyu changed the title Check in role manager python dependencies Check in role manager dependencies Jul 15, 2024
@lorenyu lorenyu merged commit 2caeef6 into main Jul 15, 2024
9 checks passed
@lorenyu lorenyu deleted the lorenyu/dbcheckindeps branch July 15, 2024 23:48
lorenyu added a commit that referenced this pull request Jul 15, 2024
lorenyu added a commit that referenced this pull request Jul 15, 2024
@lorenyu
Copy link
Collaborator Author

lorenyu commented Jul 15, 2024

Accidentally merged (my mouse was bugging out), so I reverted in #125

This was properly done as part of navapbc/template-infra#493

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.

1 participant