CurtHagenlocher commented on code in PR #2566:
URL: https://github.com/apache/arrow-adbc/pull/2566#discussion_r1977723674


##########
csharp/src/Drivers/BigQuery/BigQueryStatement.cs:
##########
@@ -126,13 +122,13 @@ public override UpdateResult ExecuteUpdate()
             QueryOptions options = ValidateOptions();
             GetQueryResultsOptions getQueryResultsOptions = new 
GetQueryResultsOptions();
 
-            if 
(this.Options?.TryGetValue(BigQueryParameters.GetQueryResultsOptionsTimeoutMinutes,
 out string? timeoutMinutes) == true)
+            if 
(this.Options?.TryGetValue(BigQueryParameters.GetQueryResultsOptionsTimeout, 
out string? timeoutSeconds) == true)
             {
-                if (int.TryParse(timeoutMinutes, out int minutes))
+                if (int.TryParse(timeoutSeconds, out int seconds))
                 {
-                    if (minutes >= 0)
+                    if (seconds >= 0)
                     {
-                        getQueryResultsOptions.Timeout = 
TimeSpan.FromMinutes(minutes);
+                        getQueryResultsOptions.Timeout = 
TimeSpan.FromMinutes(seconds);

Review Comment:
   Behaviorally, this would be a breaking change. Are we okay with that? 
   
   FWIW, the Snowflake driver allows setting timeouts as e.g "90s" or "1.5m" or 
"1m30s", which I have mixed feelings about but for which the lack of ambiguity 
around unit of measure is certainly nice.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to