exceptionfactory commented on code in PR #7570:
URL: https://github.com/apache/nifi/pull/7570#discussion_r1283774874
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/ListGCSBucket.java:
##########
@@ -396,11 +401,31 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
listByTrackingTimestamps(context, session);
} else if (BY_ENTITIES.equals(listingStrategy)) {
listByTrackingEntities(context, session);
+ } else if (NO_TRACKING.equals(listingStrategy)) {
+ listNoTracking(context, session);
} else {
throw new ProcessException("Unknown listing strategy: " +
listingStrategy);
}
}
+ private void listNoTracking(ProcessContext context, ProcessSession
session) {
+ final long startNanos = System.nanoTime();
+ final ListingAction listingAction = new
NoTrackingListingAction(context, session);
+
+ try {
+ listBucket(context, listingAction);
+ } catch (final Exception e) {
+ getLogger().error("Failed to list contents of GCS Bucket due to
{}", new Object[] {e}, e);
+ listingAction.getBlobWriter().finishListingExceptionally(e);
+ session.rollback();
+ context.yield();
+ return;
+ }
+
+ final long listMillis =
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos);
+ getLogger().info("Successfully listed GCS bucket {} in {} millis", new
Object[]{
context.getProperty(BUCKET).evaluateAttributeExpressions().getValue(),
listMillis });
Review Comment:
The `Object[]` wrapper is not necessary.
```suggestion
getLogger().info("Successfully listed GCS bucket {} in {} millis",
context.getProperty(BUCKET).evaluateAttributeExpressions().getValue(),
listMillis);
```
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/ListGCSBucket.java:
##########
@@ -396,11 +401,31 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
listByTrackingTimestamps(context, session);
} else if (BY_ENTITIES.equals(listingStrategy)) {
listByTrackingEntities(context, session);
+ } else if (NO_TRACKING.equals(listingStrategy)) {
+ listNoTracking(context, session);
} else {
throw new ProcessException("Unknown listing strategy: " +
listingStrategy);
}
}
+ private void listNoTracking(ProcessContext context, ProcessSession
session) {
+ final long startNanos = System.nanoTime();
+ final ListingAction listingAction = new
NoTrackingListingAction(context, session);
+
+ try {
+ listBucket(context, listingAction);
+ } catch (final Exception e) {
+ getLogger().error("Failed to list contents of GCS Bucket due to
{}", new Object[] {e}, e);
Review Comment:
The `due to {}` syntax for exceptions should not be used, since the message
will be incorporated in bulletin summaries and printed in stack traces.
```suggestion
getLogger().error("Failed to list contents of GCS Bucket", e);
```
--
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]
