feat(csharp/src/Drivers/BigQuery): add additional billing and timeout properties and test settings#2566
Conversation
| if (seconds >= 0) | ||
| { | ||
| getQueryResultsOptions.Timeout = TimeSpan.FromMinutes(minutes); | ||
| getQueryResultsOptions.Timeout = TimeSpan.FromMinutes(seconds); |
There was a problem hiding this comment.
TimeSpan.FromMinutes(seconds) looks sus
There was a problem hiding this 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.
There was a problem hiding this comment.
I wrestled with changing this outright vs having two options. I needed something more granular for the tests, I thought having two options would be confusing, but I decided to leave the two options for the BigQueryTestConfiguration because it's more likely a configuration file is set with timeoutMinutes and I just convert that. I dont think there are many, if any, consumers at this point, so I decided it was best to just make a clean break to seconds for the parameter.
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! The FromMinutes seems like something that needs attention.
adbc.bigquery.client.timeoutvalue for the BigQueryClientadbc.bigquery.billing_project_idvalue for the billing queries against