You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by yu-iskw <gi...@git.apache.org> on 2015/09/01 15:40:06 UTC
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
GitHub user yu-iskw opened a pull request:
https://github.com/apache/nifi/pull/80
[NIFI-774] Create a DeleteS3Object Processor
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](https://issues.apache.org/jira/browse/NIFI-774)
## Old PR
#72
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yu-iskw/nifi NIFI-774
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/80.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #80
----
commit d1dbd37629baeb9a1570f47f7583d07f54b38b5c
Author: Yu ISHIKAWA <yu...@gmail.com>
Date: 2015-07-23T04:13:15Z
[NIFI-774] Create a DeleteS3Object Processor
commit 7e6834937b01f035666b113a6c2735e4156ea4f3
Author: Yuu ISHIKAWA <yu...@gmail.com>
Date: 2015-09-01T13:36:50Z
Ignore the test suite
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-138314573
@danbress @taftster I removed the process if the key exists or not. Could you review it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-138545699
@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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-137332775
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 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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-136924301
@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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-148152423
Thank you for merging it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on a diff in the pull request:
https://github.com/apache/nifi/pull/80#discussion_r38431643
--- Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/DeleteS3Object.java ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.aws.s3;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.model.DeleteObjectRequest;
+import com.amazonaws.services.s3.model.DeleteVersionRequest;
+
+import org.apache.nifi.annotation.behavior.SupportsBatching;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.SeeAlso;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+
+@SupportsBatching
+@SeeAlso({PutS3Object.class})
+@Tags({"Amazon", "S3", "AWS", "Archive", "Delete"})
+@CapabilityDescription("Deletes FlowFiles on an Amazon S3 Bucket. " +
+ "And the FlowFiles are checked if exists or not before deleting.")
+public class DeleteS3Object extends AbstractS3Processor {
+
+ public static final PropertyDescriptor VERSION_ID = new PropertyDescriptor.Builder()
+ .name("Version")
+ .description("The Version of the Object to delete")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(true)
+ .required(false)
+ .build();
+
+ public static final List<PropertyDescriptor> properties = Collections.unmodifiableList(
+ Arrays.asList(KEY, BUCKET, ACCESS_KEY, SECRET_KEY, CREDENTAILS_FILE, REGION, TIMEOUT, VERSION_ID,
+ FULL_CONTROL_USER_LIST, READ_USER_LIST, WRITE_USER_LIST, READ_ACL_LIST, WRITE_ACL_LIST, OWNER));
+
+ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+ return properties;
+ }
+
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
--- End diff --
Yu, I do not believe you need to override ```getSupportedDynamicPropertyDescriptor``` since this processor does not use dynamic properties
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on a diff in the pull request:
https://github.com/apache/nifi/pull/80#discussion_r38919306
--- Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/DeleteS3Object.java ---
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.aws.s3;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.model.DeleteObjectRequest;
+import com.amazonaws.services.s3.model.DeleteVersionRequest;
+import org.apache.nifi.annotation.behavior.SupportsBatching;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.SeeAlso;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.util.StandardValidators;
+
+
+@SupportsBatching
+@SeeAlso({PutS3Object.class})
+@Tags({"Amazon", "S3", "AWS", "Archive", "Delete"})
+@CapabilityDescription("Deletes FlowFiles on an Amazon S3 Bucket. " +
+ "And the FlowFiles are checked if exists or not before deleting.")
+public class DeleteS3Object extends AbstractS3Processor {
+
+ public static final Relationship REL_NOT_FOUND = new Relationship.Builder().name("not found")
+ .description("FlowFiles are routed to 'not found' if it doesn't exist on Amazon S3").build();
+
+ public static final PropertyDescriptor VERSION_ID = new PropertyDescriptor.Builder()
+ .name("Version")
+ .description("The Version of the Object to delete")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(true)
+ .required(false)
+ .build();
+
+ public static final List<PropertyDescriptor> properties = Collections.unmodifiableList(
+ Arrays.asList(KEY, BUCKET, ACCESS_KEY, SECRET_KEY, CREDENTAILS_FILE, REGION, TIMEOUT, VERSION_ID,
+ FULL_CONTROL_USER_LIST, READ_USER_LIST, WRITE_USER_LIST, READ_ACL_LIST, WRITE_ACL_LIST, OWNER));
+
+ public static final Set<Relationship> relationships = Collections.unmodifiableSet(
+ new HashSet<>(Arrays.asList(REL_SUCCESS, REL_FAILURE, REL_NOT_FOUND)));
--- End diff --
REL_NOT_FOUND should not be in this list anymore
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-139494030
@danbress thank you for pointing out the easy mistake. I fixed it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-137052633
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/80
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-136828624
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 toughts
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by danbress <gi...@git.apache.org>.
Github user danbress commented on a diff in the pull request:
https://github.com/apache/nifi/pull/80#discussion_r38919291
--- Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/DeleteS3Object.java ---
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.aws.s3;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.model.DeleteObjectRequest;
+import com.amazonaws.services.s3.model.DeleteVersionRequest;
+import org.apache.nifi.annotation.behavior.SupportsBatching;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.SeeAlso;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.util.StandardValidators;
+
+
+@SupportsBatching
+@SeeAlso({PutS3Object.class})
+@Tags({"Amazon", "S3", "AWS", "Archive", "Delete"})
+@CapabilityDescription("Deletes FlowFiles on an Amazon S3 Bucket. " +
+ "And the FlowFiles are checked if exists or not before deleting.")
+public class DeleteS3Object extends AbstractS3Processor {
+
+ public static final Relationship REL_NOT_FOUND = new Relationship.Builder().name("not found")
--- End diff --
Not needed anymore
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-137064643
@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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: [NIFI-774] Create a DeleteS3Object Processor
Posted by yu-iskw <gi...@git.apache.org>.
Github user yu-iskw commented on the pull request:
https://github.com/apache/nifi/pull/80#issuecomment-136936916
@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`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---