Skip to content

Conversation

@wseaton
Copy link
Contributor

@wseaton wseaton commented Feb 25, 2024

This MR attempts to fix a bug where UUIDs are not properly regenerated on retries triggered by middleware. By moving request parameters for UUID and Timestamp generation into middleware, we fix that, but also allow future enhancements for more robust error handling modes like what is specified in: https://docs.snowflake.com/en/developer-guide/sql-api/submitting-requests#resubmitting-a-request-to-execute-sql-statements.

I still need to figure out a good solution for testing this, might end up mocking one of the endpoints.

We should be able to leverage Extension state to detect if a request is a retry, and based on the user's client configuration hint snowflake that it should "rescan" to see if the query was successful or not.

@wseaton wseaton changed the title UUID regeneration on retry Draft: UUID regeneration on retry Feb 26, 2024
@wseaton
Copy link
Contributor Author

wseaton commented Feb 26, 2024

@andrusha @dmzmk this is ready for review (no rush), I had to make some Connection API modifications to make the API client mockable. But the mock verifies the current middleware behavior, and builder methods could be added to SnowflakeApiBuilder to have it also use a mocked client which might be nice for adding tests for arrow serde or other things.

@andrusha
Copy link
Owner

The docs are for the json api, which is different from odbc api this lib implements, looking at the https://github.com/snowflakedb/gosnowflake/blob/2f775957a87540a93078051bc132991777ec715a/retry.go#L49 they do pass a similar parameter however, but it's retryCount instead, so it'll be useful

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.

2 participants