Skip to content

Conversation

@vdonato
Copy link
Collaborator

@vdonato vdonato commented Apr 11, 2023

📚 Context

This PR adds simple retry behavior to the read_sql helper methods of the SQL and Snowpark
connectors. We rely on the tenacity library to implement this, which we add as a "dependency" only
when actually using a connection.

For now, the behavior is quite simplistic:

  • We currently retry regardless of the type of the exception thrown
    • This will be improved before this feature branch is merged into develop
  • We attempt to retry 3 times, waiting 1 second between attempts
    • This is potentially something we may improve before moving this out of the experimental_
      namespace.
  • After a failed call, we reset the connection entirely so that the next
    attempt to use recreates the underlying connection object.

@vdonato vdonato added the security-assessment-completed Security assessment has been completed for PR label Apr 11, 2023
@vdonato vdonato requested a review from kajarenc April 11, 2023 00:34
@vdonato vdonato marked this pull request as ready for review April 11, 2023 00:46
@vdonato vdonato requested a review from lukasmasuch April 11, 2023 18:31
@vdonato vdonato force-pushed the feature/st.experimental_connection branch from 631a509 to ccf92e6 Compare April 11, 2023 19:08
@vdonato vdonato force-pushed the vdonato/connection-read-method-retries branch 2 times, most recently from 04f40f2 to 69b6940 Compare April 11, 2023 22:31
@vdonato vdonato force-pushed the vdonato/connection-read-method-retries branch from 69b6940 to d0a5403 Compare April 11, 2023 22:34
@kajarenc
Copy link
Collaborator

LGTM! 👍

@vdonato vdonato merged commit 8025600 into feature/st.experimental_connection Apr 12, 2023
@vdonato vdonato deleted the vdonato/connection-read-method-retries branch April 12, 2023 17:45
vdonato added a commit that referenced this pull request Apr 15, 2023
* Add tenacity as an "optional dependency" for the library (but a dependency for connections)

* Add retries to SQL.read_sql

* Add retries to Snowpark.read_sql

* Fix tests failing due to rebase
vdonato added a commit that referenced this pull request Apr 17, 2023
* Add tenacity as an "optional dependency" for the library (but a dependency for connections)

* Add retries to SQL.read_sql

* Add retries to Snowpark.read_sql

* Fix tests failing due to rebase
vdonato added a commit that referenced this pull request Apr 19, 2023
* Add tenacity as an "optional dependency" for the library (but a dependency for connections)

* Add retries to SQL.read_sql

* Add retries to Snowpark.read_sql

* Fix tests failing due to rebase
vdonato added a commit that referenced this pull request Apr 20, 2023
* Add tenacity as an "optional dependency" for the library (but a dependency for connections)

* Add retries to SQL.read_sql

* Add retries to Snowpark.read_sql

* Fix tests failing due to rebase
vdonato added a commit that referenced this pull request Apr 21, 2023
* Add tenacity as an "optional dependency" for the library (but a dependency for connections)

* Add retries to SQL.read_sql

* Add retries to Snowpark.read_sql

* Fix tests failing due to rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants