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

Default connect_timeout to None #433

Merged
merged 6 commits into from
May 24, 2023

Conversation

jiezhen-chen
Copy link
Contributor

@jiezhen-chen jiezhen-chen commented May 8, 2023

resolves #427

Description

Change the default of connect_timeout to None. If connect_timeout is not provided in profiles.yml, connect_timeout will be defaulted to None. User can also configure connect_timeout in profiles.yml to the duration of seconds before connection times out.

Checklist

@jiezhen-chen jiezhen-chen requested a review from a team as a code owner May 8, 2023 16:34
@cla-bot cla-bot bot added the cla:yes label May 8, 2023
@dbeatty10 dbeatty10 added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label May 8, 2023
Copy link
Contributor

@McKnight-42 McKnight-42 left a comment

Choose a reason for hiding this comment

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

Making this the same by default as the redshift connector seems like a good play, users can also shorten it if they wish within the profile. thank you for all your work into this.

@McKnight-42 McKnight-42 merged commit c12b625 into dbt-labs:main May 24, 2023
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

Thank you @jiezhen-chen !

@McKnight-42 McKnight-42 added backport 1.5.latest and removed backport 1.5.latest ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering labels May 24, 2023
@McKnight-42 McKnight-42 mentioned this pull request May 24, 2023
6 tasks
mirnawong1 added a commit to dbt-labs/docs.getdbt.com that referenced this pull request Jun 7, 2023
…timeout (#3456)

## What are you changing in this pull request and why?

This PR is to update the docs for the following changes in dbt-redshift:

1. **[Mapping sslmode in psycopg2 to ssl & sslmode in
redshift_connector](dbt-labs/dbt-redshift#439

In dbt-redshift 1.5, Python driver switched from psycopg2 to
redshift_connector. redshift_connector has both ssl and sslmode
parameters, while psycopg2 only has sslmode. We've made changes in
dbt-redshift to convert psycopg2' accepted values of sslmode into ssl
and sslmode parameters of redshift_connector. This PR explains the
conversion logic and includes explanation of the sslmode flag.
  #3365

2. **[Adding `autocommit` as a
parameter](dbt-labs/dbt-redshift#475

Autocommit is a new flag that is defaulted to True. This PR adds
explanation of this change and rationale behind this decision in dbt
docs
    #3401

3. **[Connect_timeout default to
None](dbt-labs/dbt-redshift#433
  
Connect_timeout had the default of 10 seconds previously. We changed
this default to None.
    #3460




<!---
Describe your changes and why you're making them. If linked to an open
issue or a pull request on dbt Core, then link to them here! 

To learn more about the writing conventions used in the dbt Labs docs,
see the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md).
-->

## Checklist
<!--
Uncomment if you're publishing docs for a prerelease version of dbt
(delete if not applicable):
- [ ] Add versioning components, as described in [Versioning
Docs](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#versioning-entire-pages)
- [ ] Add a note to the prerelease version [Migration
Guide](https://github.com/dbt-labs/docs.getdbt.com/tree/current/website/docs/guides/migration/versions)
-->
- [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.
- [ ] Add a checklist item for anything that needs to happen before this
PR is merged, such as "needs technical review" or "change base branch."
abbywh pushed a commit to abbywh/dbt-redshift that referenced this pull request Oct 11, 2023
* change connect_timeout to defaulted None

* update test_redshift_adapter to pass new changes

* test that connect_timeout parameter is called with the right value

* add new changelog

---------

Co-authored-by: Matthew McKnight <91097623+McKnight-42@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants