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

Full redshift connector parameter support #559

Conversation

christopherscholz
Copy link

resolves #332
docs dbt-labs/docs.getdbt.com/#
alternative for #323

Problem

Currently not all connection methods, which the redshift_connector supports are currently possible with dbt-redshift.
Also the connections.py does not fully reflect the move to redshift_connector yet. In the past the connection methods were handled by dbt-redshift itself, by parsing different credentials sets based on the connections method. With redshift_connector this should now be done by redshift_connector.

Solution

I have added all connection parameters from redshift_connector and removed the ConnectionFactory, as all connection methods are handled by redshift_connector now.

Many unit tests are now obsolete, as testing the different connection methods are now out of scope for dbt-redshift, but part of redshift_connector.

All the changes should be backwards compatible.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
    Lots new adapter credentials options

@christopherscholz christopherscholz requested a review from a team as a code owner July 30, 2023 16:35
@cla-bot cla-bot bot added the cla:yes label Jul 30, 2023
@christopherscholz
Copy link
Author

Both unit and integration tests are successful for me.
Unfortunately I cannot tests all the different connection methods (as I don't have those set up currently), but as this is part of redshift_connector anyway; I guess this out of scope.

@halilduygulu
Copy link

This is great. Currently we need to set client_protocol_version to 0 due to this Teleport limitation and looking forward to see this PR merged.

@misteliy
Copy link

Any updates here or what is currently blocking?

@halilduygulu
Copy link

+1 to @misteliy . Why are you not merging this pr?

Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 19, 2024
Copy link
Contributor

Although we are closing this PR as stale, it can still be reopened to continue development. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this May 26, 2024
@misteliy
Copy link

Could you please reopen as it was not resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants