markap14 commented on a change in pull request #5288:
URL: https://github.com/apache/nifi/pull/5288#discussion_r689843591
##########
File path:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java
##########
@@ -333,4 +338,47 @@ protected final CannedAccessControlList
createCannedACL(final ProcessContext con
return cannedAcl;
}
+
+ @Override
+ public List<ConfigVerificationResult> verify(final ProcessContext context,
final ComponentLog logger, final Map<String, String> attributes) {
+ final AmazonS3Client client = createClient(context,
getCredentials(context), createConfiguration(context));
+ initializeRegionAndEndpoint(context, client);
+
+ final List<ConfigVerificationResult> results = new ArrayList<>();
+ final String bucketName =
context.getProperty(BUCKET).evaluateAttributeExpressions(attributes).getValue();
+
+ if (bucketName == null || bucketName.trim().isEmpty()) {
+ results.add(new ConfigVerificationResult.Builder()
+ .verificationStepName("Perform Listing")
+ .outcome(Outcome.FAILED)
+ .explanation("Bucket Name must be specified")
+ .build());
+
+ return results;
+ }
+
+ // Attempt to perform a listing of objects in the S3 bucket
+ try {
+ final ObjectListing listing = client.listObjects(bucketName);
Review comment:
Yeah that's a good point about permissions. I think what makes the most
sense here is probably to move this from AbstractS3Processors to ListS3. I do
believe it makes sense to perform the actual listing and determine how many
objects are in the bucket, though. There are a couple of reasons for this.
Firstly, if it's misconfigured you could end up attempting to get the listing
for something like empty string - if you try that, no error. It returns
successfully, and there will be 0 objects listed. So the fact that the listing
came back with 0 objects helps to make it obvious that something is wrong.
Also, if you perform a listing and expect 3 things in the bucket but get
thousands (or vice versa) that can help to alert you that maybe you are
configured for wrong bucket. I can definitely see a situation where a user is
expecting to list a bucket with a few elements and enters the wrong bucket name
because they have many buckets, and then they end up with a huge listing when
they
run the processor, and I think this will help there.
For the scope of this PR, I think what makes most sense is to just move this
into ListS3. We can then iterate once these changes are merged and improve each
of the processors. For this PR, I just wanted to pick a couple of components to
use as a proof of concept, basically.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
