CurtHagenlocher commented on code in PR #2810:
URL: https://github.com/apache/arrow-adbc/pull/2810#discussion_r2084853852
##########
csharp/src/Drivers/BigQuery/BigQueryStatement.cs:
##########
@@ -93,27 +93,8 @@ private async Task<QueryResult> ExecuteQueryInternalAsync()
getQueryResultsOptions.Timeout = TimeSpan.FromSeconds(seconds);
}
- Func<Task<BigQueryJob>> checkJobStatus = async () =>
- {
- while (true)
- {
- var jobWithStatus = await Client.GetJobAsync(jobReference);
-
- if (jobWithStatus.State == JobState.Done)
- {
- if (jobWithStatus.Status.ErrorResult != null)
- {
- // TODO: log
- Debug.WriteLine($"Error:
{jobWithStatus.Status.ErrorResult.Message}");
- }
-
- return jobWithStatus;
- }
- }
- };
-
- await ExecuteWithRetriesAsync<BigQueryJob>(checkJobStatus);
-
+ // We can't checkJobStatus, Otherwise, the timeout in
QueryResultsOptions is meaningless.
Review Comment:
Would removing the polling loop not break the case where there's a
longer-running job?. To the extent that there's a problem with the query
timeout, should that not instead be reflected in the "while true" logic that
doesn't currently take a timeout into account?
@davidhcoe, waiting for your feedback on this.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
