[NIFI-774] Create a DeleteS3Object Processor

[yu-iskw]

I'm sorry for the delay of my sending a new PR again. @danbress could you review it?

JIRA

[NIFI-774] Create a DeleteS3Object Processor - ASF JIRA

Old PR

#72

[danbress]

Yu, I do not believe you need to override getSupportedDynamicPropertyDescriptor since this processor does not use dynamic properties

[danbress]

Yu, this looks great. Thanks again for sticking with it and contributing. I made one comment inline.

I have one other comment that I'd like to get others feedback on:
This processor attempts to delete data from Amazon S3. I could see there being a case for three scenarios, and three relationships out of the processor

1. the data is found in S3, and deleted(success)
2. the data is not found in S3, and therefore not deleted(not found)
3. there was some issue communicating with S3 and the object could not be deleted(failure)

Basically, you would be adding a "not found" relationship, and routing FlowFiles there when you they are not found in S3 before you attempt to delete them. The reason being, you allow the user of your processor the opportunity to treat "not found" situations differently from "failure" situations.

I tried to find other cases like this in the NiFi processor marketplace, but I couldn't. (https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/sqs/DeleteSQS.java) is the only "delete" processor I see, and it doesn't behave the way I am describing.

@markap14 @mcgilman @joewitt thoughts?

[yu-iskw]

@danbress thank you for the comment. That sounds good. It would be nice to add a property in order for users to handle if "not found" raises an error or not.

[yu-iskw]

@danbress could you review it?

• Transfer REL_NOT_FOUND if not found target object on Amazon S3
• Add REL_NOT_FOUND to DeleteS3Object
• Redefine relationships on DeleteS3Object to append REL_NOT_FOUND
• Instead, get rid of final from AbstractS3Processor.relationships

[danbress]

Yu, looks good to me. 0.3.0 is being tested right now, as soon as it is tested and released I will merge these changes in. Thanks again for your contribution.

[yu-iskw]

@danbress thank you for reviewing it. Sorry, one more thing, I fixed a typo and modified an error message. I look forward to merging it.

[taftster]

I'm not sure that checking for the existence of an existing object is that important. At minimum, it's causing an extra call to the remote S3 service to determine if the object exists. The S3 service itself will not return a different response code (edit: for the DELETE call) if the object previously existed.

What value is added by having the REL_NOT_FOUND relationship? What use case is this trying to solve?

A request to delete an object, regardless of its previous existence, still results in the same remote state. HTTP DELETE, being an idempotent method, doesn't really care if the object existed beforehand or not.

[yu-iskw]

@danbress @taftster I removed the process if the key exists or not. Could you review it?

[danbress]

Not needed anymore

[danbress]

@yu-iskw Looks great! Only thing I see right now is that the REL_NOT_FOUND relationship isn't needed anymore. Thanks again for the contribution. I will try and merge this in as soon as 0.3.0 is released.

[yu-iskw]

@danbress thank you for pointing out the easy mistake. I fixed it.

[yu-iskw]

Thank you for merging it!