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

Allow IAM username suffix to be added to S3 scratch URL #69

Merged
merged 17 commits into from
Jun 2, 2020

Conversation

vinceatbluelabs
Copy link
Contributor

@vinceatbluelabs vinceatbluelabs commented May 28, 2020

This PR retires the scratch-s3-bucket script and introduces the ability to use an IAM username as part of the scratch bucket URL with appropriate config files in place. I'll work to get those set up so we can release this and have it work for our internal use.

I also moved the scratch bucket calculation over to BaseCreds, and renamed the functions to include the word 'default'. Your AWS IAM username depends on which AWS creds you are using. We always use the default AWS creds when loading/unloading on Redshift. That might not be true in the future, and once we provide more flexibility there, we'll want to extend this code to take in the appropriate credentials name for the AWS creds.

@vinceatbluelabs vinceatbluelabs changed the title Delegate scratch_s3_url inference to Creds hierarchy Allow IAM username suffix to be added to S3 scratch URL May 29, 2020
@@ -1 +1 @@
981
982
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment to setup.py to explain a new version constraint.

@@ -1,7 +1,9 @@
from db_facts.db_facts_types import DBFacts
from .database import db_facts_from_env
from typing import TYPE_CHECKING, Iterable, Union, Optional
from records_mover.mover_types import NotYetFetched
from records_mover.mover_types import PleaseInfer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I standardized on the latter here; the difference in semantics was so subtle as to be meaningless for the uses here.

@vinceatbluelabs vinceatbluelabs marked this pull request as ready for review May 29, 2020 15:04
@vinceatbluelabs vinceatbluelabs requested a review from cwegrzyn May 29, 2020 15:05
Copy link
Contributor

@cwegrzyn cwegrzyn left a comment

Choose a reason for hiding this comment

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

Nice!

@vinceatbluelabs vinceatbluelabs merged commit 4afb9cd into master Jun 2, 2020
@vinceatbluelabs vinceatbluelabs deleted the scratch_s3_url_in_creds_interface branch June 2, 2020 19:55
vinceatbluelabs added a commit that referenced this pull request Jun 2, 2020
This will allow folks using custom cred types (like LastPass!) to configure this globally via config files. Combined with #69 this will allow enterprise uses to be configured while more generic settings are used by default.
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.

3 participants