chamikaramj commented on code in PR #40233:
URL: https://github.com/apache/beam/pull/40233#discussion_r4097109705
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java:
##########
@@ -1734,6 +1746,12 @@ static <T> T executeWithRetries(
return request.execute();
} catch (IOException e) {
lastException = e;
+ if (ApiErrorExtractor.INSTANCE.badRequest(e)
+ && e.getMessage() != null
+ && e.getMessage().contains(BIGQUERY_NOT_ENABLED)) {
+ LOG.error(BIGQUERY_NOT_ENABLED_GUIDANCE, e);
+ throw new IOException(errorMessage + " " +
BIGQUERY_NOT_ENABLED_GUIDANCE, e);
Review Comment:
This includes the caller supported error and actual error from BQ
(`e.getMessage()`) might be hidden in a long stack-trace. Can we also make
`e.getMessage()` in the exception raised.
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImplTest.java:
##########
@@ -1523,6 +1523,63 @@ public void testInsertTimeoutLog() throws Exception {
verifyWriteMetricWasSet("project", "dataset", "table", " no rows present
in the request. ", 1);
}
+ /**
+ * Tests that {@link DatasetServiceImpl#insertAll} logs and includes
suggested remedy when the
Review Comment:
Can you add a unit test for `BigQueryServicesImpl.executeWithRetries` that
confirms the proper behavior for the exception raised.
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java:
##########
@@ -1734,6 +1746,12 @@ static <T> T executeWithRetries(
return request.execute();
} catch (IOException e) {
lastException = e;
+ if (ApiErrorExtractor.INSTANCE.badRequest(e)
+ && e.getMessage() != null
+ && e.getMessage().contains(BIGQUERY_NOT_ENABLED)) {
+ LOG.error(BIGQUERY_NOT_ENABLED_GUIDANCE, e);
+ throw new IOException(errorMessage + " " +
BIGQUERY_NOT_ENABLED_GUIDANCE, e);
+ }
Review Comment:
Also the provided `errorMessage` "aborting after %d retries." will be wrong
in this case since we are failing fast after an exception.
--
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]
