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

Added section about profiles on the page about connecting to redshif via IAM #238

Merged
merged 4 commits into from
Jul 13, 2020
Merged

Added section about profiles on the page about connecting to redshif via IAM #238

merged 4 commits into from
Jul 13, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 25, 2020

Added section about profiles on the page about connecting to redshif via IAM

Description & motivation

This is linked to the PR #2851, clarifying how a user would connect to Redshift using the IAM method with a profile specified

@drewbanin drewbanin self-requested a review June 25, 2020 17:56
@brunomurino
Copy link
Contributor

Hey @drewbanin, care to review this please? ;)

Copy link
Collaborator

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

Hey @landbay-brunomurino - this is great! The feature you're documenting has not been released yet, so I think we might want to call out the versions that support this feature in these docs.

Check out the notes below for some additional thoughts. Thanks for opening this PR!

@@ -71,6 +72,31 @@ my-redshift-db:

</File>

Ideally, the region and access keys should be configured in the usual `~/.aws` folder,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a header above this line like:

### Specifying an IAM Profile

:::info New in dbt v0.18.0
The `iam_profile` config option for Redshift profiles is new in dbt v0.18.0
:::

<File name='~/.aws/config.yml'>

```
[default] # 'default' profile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the idea here that the data_engineer profile above is pointing back to the default profile for access keys? I think I don't totally understand the purpose of these docs, and I question if they're relevant enough to dbt to include here. If there's an external resource we can link out to that provides best practices around how to configure an .aws/config file, I'd be supportive of linking out to it here.

@brunomurino
Copy link
Contributor

@drewbanin Done! I ended up just referring to the documentation as it's already pretty rich. Thanks for the review!

Copy link
Collaborator

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

Ok - one more quick suggestion here that will improve these docs a little, and then I think this PR is ready to ship! Thanks for your patience on this one :)

@@ -42,7 +42,8 @@ on generating user credentials with IAM Auth.

If you receive the "You must specify a region" error when using IAM
Authentication, then your aws credentials are likely misconfigured. Try running
`aws configure` to set up AWS access keys, and pick a default region.
`aws configure` to set up AWS access keys, and pick a default region. If you have any questions,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

website/docs/docs/supported-databases/profile-redshift.md Outdated Show resolved Hide resolved
Co-authored-by: Drew Banin <drew@fishtownanalytics.com>
@brunomurino
Copy link
Contributor

@drewbanin Should be ready now, no?

@drewbanin
Copy link
Collaborator

@brunomurino this is unfortunate, but I think the profile-redshift.md file was changed out from under us in this PR :(

Are you able to address the merge conflict? We shuffled some files around - I think we might want to move these changes over to this file after the restructuring.

Sorry about that, and let me know if I can help out at all with this one

@ghost ghost requested a review from clrcrl as a code owner July 11, 2020 12:14
@clrcrl clrcrl removed their request for review July 13, 2020 13:30
@clrcrl
Copy link
Contributor

clrcrl commented Jul 13, 2020

Removing myself as a reviewer since Drew has this one covered :)

@drewbanin drewbanin merged commit 23b946d into dbt-labs:master Jul 13, 2020
@drewbanin
Copy link
Collaborator

Thanks @brunomurino - merged!

mirnawong1 pushed a commit that referenced this pull request Oct 5, 2023
REPO SYNC - Public to Private
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