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

Remove ENV["USE_NEW_AWS"] and use Orig ENV variables for New Creds #5771

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

mstruve
Copy link
Contributor

@mstruve mstruve commented Jan 27, 2020

What type of PR is this? (check all applicable)

  • Refactor

Description

Now that we are pointed at our new AWS bucket swap over the original ENV variables to the new bucket ones and use those. The only variable we have to keep is the region since that is different than our default region and that default region is being used by the SDK which is used by the AWS_LAMBDA client.

The ENV variables have all been set properly in Heroku.
AWS_SECRET = AWS_UPLOAD_SECRET
AWS_ID = AWS_UPLOAD_ID
AWS_BUCKET_NAME = AWS_UPLOAD_BUCKET_NAME

Related Tickets & Documents

#5570

Added to documentation?

  • no documentation needed

alt_text

@mstruve mstruve requested a review from a team January 27, 2020 19:00
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Jan 27, 2020
@mstruve mstruve requested review from benhalpern, atsmith813, joshpuetz and rhymes and removed request for benhalpern January 27, 2020 19:06
atsmith813
atsmith813 previously approved these changes Jan 27, 2020
Copy link
Contributor

@atsmith813 atsmith813 left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jan 27, 2020
Copy link
Contributor

@rhymes rhymes left a comment

Choose a reason for hiding this comment

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

Aside from fixing the default value of the environment variable I think we're there!

Thanks for the transition!

Envfile Outdated
@@ -73,6 +67,7 @@ variable :ALGOLIASEARCH_SEARCH_ONLY_KEY, :String, default: only_in_test
variable :AWS_ID, :String, default: "Optional"
variable :AWS_SECRET, :String, default: "Optional"
variable :AWS_BUCKET_NAME, :String, default: "Optional"
variable :AWS_UPLOAD_REGION, :String, default: "Optional"
Copy link
Contributor

Choose a reason for hiding this comment

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

this has to be an empty string, for the OR in the carrierwave initializer and the site map to work. Otherwise the app locally will boot with "Optional" as the region

Optional is just an arbitrary string I hope we can get rid of :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh snap! Good catch!

@pr-triage pr-triage bot added PR: reviewed-changes-requested bot applied label for PR's where reviewer requests changes and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Jan 27, 2020
@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: reviewed-changes-requested bot applied label for PR's where reviewer requests changes labels Jan 27, 2020
@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jan 27, 2020
@benhalpern benhalpern merged commit b06ad50 into master Jan 27, 2020
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Jan 27, 2020
@maestromac maestromac deleted the mstruve/use-orig-aws-vars branch January 27, 2020 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants