Pass region param on to redshift conn as is
#485
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the dbt-redshift contributing guide. |
colin-rogers-dbt
changed the title
region in the hostname
dbeatty10
reviewed
Jun 12, 2023
colin-rogers-dbt
changed the title
region in the hostnameregion param on to redshift conn as is
|
Loving how this is looking @colin-rogers-dbt 🤩 Since we're removing the region logic entirely in 2dfa187, we'll need to open an issue to update docs for this page: Specifically, to make this change in a couple different places: - region: # optional, if not provided, will be determined from host (e.g. host.123.us-east-1.redshift-serverless.amazonaws.com)
+ region: # optional |
1 task
|
As a follow-up to #485 (comment), opened up a PR to update the docs here: |
|
👋 Hi. A few questions:
|
This change is specifically for customers (like those using the Cloud IDE) who cannot pass the region parameter at this time.
For now you will not provide this parameter.
The region parameter is only required if using IAM auth so for Cloud IDE users this poses no risk. This change has been made after discussion with the maintainers of redshift-connector (where they are also doing this fallback inference in some places) with the decision that we don't want to maintain logic that should be handled by that package. |
dataders
added a commit
to dbt-labs/docs.getdbt.com
that referenced
this pull request
Jun 14, 2023
Previews: - [Password-based authentication](https://deploy-preview-3519--docs-getdbt-com.netlify.app/docs/core/connect-data-platform/redshift-setup#password-based-authentication) - [IAM Authentication](https://deploy-preview-3519--docs-getdbt-com.netlify.app/docs/core/connect-data-platform/redshift-setup#iam-authentication) ## What are you changing in this pull request and why? dbt-labs/dbt-redshift#485 is updating handling of the `region` parameter for dbt-redshift to be completely optional without additional side-effects (like attempting to parse it from the host name). ## Checklist - [x] Review the [Content style guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md) and [About versioning](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#adding-a-new-version) so my content adheres to these guidelines.
McKnight-42
approved these changes
Jun 14, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Jun 15, 2023
* convert test_store_test_failures to functional test * test not requiring a region in hostname * remove dev-requirements.txt change * add unit test and clean up logic * add changie * simplify region inference logic * remove region logic entirely (cherry picked from commit cbb45ca)
colin-rogers-dbt
added a commit
that referenced
this pull request
Jun 15, 2023
* convert test_store_test_failures to functional test * test not requiring a region in hostname * remove dev-requirements.txt change * add unit test and clean up logic * add changie * simplify region inference logic * remove region logic entirely (cherry picked from commit cbb45ca) Co-authored-by: colin-rogers-dbt <[email protected]>
6 tasks
abbywh
pushed a commit
to abbywh/dbt-redshift
that referenced
this pull request
Oct 11, 2023
* convert test_store_test_failures to functional test * test not requiring a region in hostname * remove dev-requirements.txt change * add unit test and clean up logic * add changie * simplify region inference logic * remove region logic entirely
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #484 resolves #483
Description
Region is a non-obvious semi-requirement post 1.5 (you don't need to specify it if it's in your hostname), this has been a headache for various users and it's not clear that it's strictly speaking necessary.
This change makes it a truly optional requirement for dbt-redshift, this removes duplication with the underlying redshift-connector to properly manage and validate this parameter.
Checklist
changie newto create a changelog entry