andrusha / snowflake-rs

Snowflake API libraries for Rust
Apache License 2.0
31 stars 26 forks source link

Draft: UUID regeneration on retry #26

Open wseaton opened 9 months ago

wseaton commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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