You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "exceptionfactory (via GitHub)" <gi...@apache.org> on 2023/08/03 22:22:54 UTC
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7570: NIFI-11891 - add no tracking listing strategy to ListGCS
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org