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.

Sign up for GitHub

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

Jump to bottom

Style improvements in deploy AWS python scripts #3128

Merged
merged 3 commits into from Aug 11, 2022
Merged

Style improvements in deploy AWS python scripts #3128

merged 3 commits into from Aug 11, 2022

Conversation

nbeloglazov
Copy link
Contributor

Description

  1. Pass command line as a string instead of list of parameters as it's easier to read that way.
  2. Extract s3 bucket name to a constant in resources.py so that more terraform-related constants in that file.
  3. Move code that builds urls for AWS console to AwsCli.

Checklist

  • Added the correct label: < feature | bug | dependencies | infrastructure | ignore-for-release >
  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary

@nbeloglazov nbeloglazov added infrastructure Setting up tooling, deployment, cloud, etc deploy Work for the deploy directory or to enable new deployments labels Aug 10, 2022
@nbeloglazov nbeloglazov self-assigned this Aug 10, 2022
@nbeloglazov
Copy link
Contributor Author

Terraform failure is unrelated to this PR. See open bug: hashicorp/terraform#31615

azizova-leyla
azizova-leyla suggested changes Aug 11, 2022
View reviewed changes
cloud/aws/templates/aws_oidc/bin/aws_cli.py Outdated

def get_current_user(self) -> str:
return self._call_cli(['sts', 'get-caller-identity'])['UserId']
return self._call_cli('sts get-caller-identity')['UserId']
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be just 'sts get-caller-identity UserId' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_call_cli returns a dictionary (JSON) so we need to get UserId from it.

cloud/aws/templates/aws_oidc/bin/destroy.py Outdated Show resolved Hide resolved
@nbeloglazov nbeloglazov enabled auto-merge (squash) August 11, 2022 16:27
@nbeloglazov nbeloglazov merged commit 9765fe6 into main Aug 11, 2022
@nbeloglazov nbeloglazov deleted the mikita/improve-deploy-code branch August 11, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azizova-leyla azizova-leyla azizova-leyla approved these changes

Assignees

@nbeloglazov nbeloglazov

Labels
deploy Work for the deploy directory or to enable new deployments infrastructure Setting up tooling, deployment, cloud, etc
Projects
CiviForm Project Roadmap & Tracker
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@nbeloglazov @azizova-leyla