You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by MikeThomsen <gi...@git.apache.org> on 2018/04/07 10:09:56 UTC
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
GitHub user MikeThomsen opened a pull request:
https://github.com/apache/nifi/pull/2616
NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MikeThomsen/nifi NIFI-5052
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2616.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 #2616
----
commit c179eb57490cb41c8d83e698407788dd4df8ed5d
Author: Mike Thomsen <mi...@...>
Date: 2018-04-07T10:03:01Z
NIFI-5052 Added DeleteByQuery ElasticSearch processor.
----
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180870855
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml ---
@@ -137,7 +149,7 @@
<httpPort>9400</httpPort>
<version>5.6.2</version>
<timeout>90</timeout>
- <pathInitScript>${project.basedir}/src/test/resources/setup.script</pathInitScript>
+ <!--<pathInitScript>${project.basedir}/src/test/resources/setup.script</pathInitScript>-->
--- End diff --
Done.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2616
Sorry I lost track of this, was reviewing your other ones. I'll try to take a look ASAP
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180815675
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml ---
@@ -69,11 +69,11 @@
<version>2.6</version>
</dependency>
- <dependency>
- <groupId>org.elasticsearch.client</groupId>
- <artifactId>rest</artifactId>
- <version>5.0.1</version>
- </dependency>
+ <!--<dependency>-->
+ <!--<groupId>org.elasticsearch.client</groupId>-->
+ <!--<artifactId>rest</artifactId>-->
+ <!--<version>5.6.6</version>-->
+ <!--</dependency>-->
--- End diff --
Comments to be deleted?
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@zenfenan sounds like a plan. After the change to `expressionLanguageSupport` I might have to rebase this today FYI.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@mattyb149 ready to merge?
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180870831
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml ---
@@ -69,11 +69,11 @@
<version>2.6</version>
</dependency>
- <dependency>
- <groupId>org.elasticsearch.client</groupId>
- <artifactId>rest</artifactId>
- <version>5.0.1</version>
- </dependency>
+ <!--<dependency>-->
+ <!--<groupId>org.elasticsearch.client</groupId>-->
+ <!--<artifactId>rest</artifactId>-->
+ <!--<version>5.6.6</version>-->
+ <!--</dependency>-->
--- End diff --
Done
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180815792
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml ---
@@ -137,7 +149,7 @@
<httpPort>9400</httpPort>
<version>5.6.2</version>
<timeout>90</timeout>
- <pathInitScript>${project.basedir}/src/test/resources/setup.script</pathInitScript>
+ <!--<pathInitScript>${project.basedir}/src/test/resources/setup.script</pathInitScript>-->
--- End diff --
Same here. Can they be removed?
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180870902
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
--- End diff --
Done.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2616
I'm extremely sorry for the delay. Got held up by some unexpected works. Will try to complete by this week. Is that okay?
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r184432659
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml ---
@@ -120,6 +115,18 @@
<version>1.7.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.elasticsearch</groupId>
+ <artifactId>elasticsearch</artifactId>
+ <version>5.6.8</version>
+ <scope>compile</scope>
--- End diff --
Not sure these need to be explicitly scoped? But if it works, no harm no foul...
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r184851877
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/test/java/org/apache/nifi/elasticsearch/integration/ElasticSearchClientService_IT.java ---
@@ -102,4 +129,43 @@ public void testBasicSearch() throws Exception {
Assert.assertEquals(String.format("%s did not match", key), expected.get(key), docCount);
}
}
+
+ @Test
+ public void testDeleteByQuery() throws Exception {
+ String query = "{\n" +
--- End diff --
Done.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2616
@MikeThomsen Nope. I don't have commit privileges.
BTW, I have ran the build and tested locally.
- Tested the new `DeleteByQueryElasticsearch` processor with both flowfile content as well as `Query` property
- Tested the `JsonQueryElasticsearch` to make sure that the new changes doesn't cause ant new issues to it.
Everything works fine and looks good to me. Thanks, @MikeThomsen
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
Anything @zenfenan?
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/2616
Hey @MikeThomsen sorry to leave you hanging but I just got super busy with my day job. I may be able to help in a couple weeks can't at the moment.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@mattyb149 can we close this out? It's blocking me from fleshing out the rest of the CRUD capabilities around the new client service.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2616
+1 LGTM, ran the build with tests and contrib-check, also tested various situations (with/out incoming connections, valid/invalid queries, etc.) and all looks good. Thanks for this feature! Merging to master
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r184851810
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/DeleteByQueryElasticsearch.java ---
@@ -0,0 +1,145 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.annotation.lifecycle.OnScheduled;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.elasticsearch.DeleteOperationResponse;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.AbstractProcessor;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_ALLOWED)
+@CapabilityDescription("Delete from an ElasticSearch index using a query. The query can be loaded from a flowfile body " +
+ "or from the Query parameter.")
+@Tags({ "elastic", "elasticsearch", "delete", "query"})
+@WritesAttributes({
+ @WritesAttribute(attribute = "elasticsearch.delete.took"),
--- End diff --
Done.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@zenfenan Done. Do you have commit privileges? Had to ask because I'm not sure how up to date the team page is on apache.org.
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180815175
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
+ .required(true)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor TYPE = new PropertyDescriptor.Builder()
+ .name("el-rest-type")
+ .displayName("Type")
+ .description("The type of this document (used by Elasticsearch for indexing and searching)")
+ .required(false)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor QUERY = new PropertyDescriptor.Builder()
+ .name("el-rest-query")
+ .displayName("Query")
+ .description("A query in JSON syntax, not Lucene syntax. Ex: " +
--- End diff --
`description` can be more descriptive. If we don't give the query in `QUERY` then it reads from the flowfile content. I think that can be added to the description.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@mattyb149 @pvillard31 Got any time for a review?
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2616
Reviewing...
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180822675
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
+ .required(true)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor TYPE = new PropertyDescriptor.Builder()
+ .name("el-rest-type")
+ .displayName("Type")
+ .description("The type of this document (used by Elasticsearch for indexing and searching)")
+ .required(false)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor QUERY = new PropertyDescriptor.Builder()
+ .name("el-rest-query")
+ .displayName("Query")
+ .description("A query in JSON syntax, not Lucene syntax. Ex: " +
+ "{\n" +
--- End diff --
Same here - not something you introduced but moved and mostly a cosmetic change. Feel free to ignore this. The `\t` and `\n` won't be rendered in the UI so simply writing `{ \"query\": { \"match\": { \"name\": \"John Smith\" } } }"` is enough.
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r184433040
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/test/java/org/apache/nifi/elasticsearch/integration/ElasticSearchClientService_IT.java ---
@@ -102,4 +129,43 @@ public void testBasicSearch() throws Exception {
Assert.assertEquals(String.format("%s did not match", key), expected.get(key), docCount);
}
}
+
+ @Test
+ public void testDeleteByQuery() throws Exception {
+ String query = "{\n" +
--- End diff --
Nitpick, but if ES doesn't require the query be pretty-printed, it might be easier to read if it were just a one-line string.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@JPercivall if you have any time, do you think you could take a look at this?
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180821403
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
--- End diff --
I know this is not introduced by you, rather moved over from `JsonQueryElasticSearch` but for `JsonQueryElasticSearch`, the description matched the intention, because it was just reading but since now the delete processor is using this property, changing it to something like "The name of the ElasticSearch Index to use" makes sense
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2616
Reviewing..
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
@mattyb149 made those changes.
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
Thanks and np. We need more committers :)
---
[GitHub] nifi issue #2616: NIFI-5052 Added DeleteByQuery ElasticSearch processor.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2616
Thanks @zenfenan
@mattyb149 @pvillard31 What do you think? Ready to merge?
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r181066059
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -51,14 +51,8 @@
PropertyDescriptor QUERY = new PropertyDescriptor.Builder()
.name("el-rest-query")
.displayName("Query")
- .description("A query in JSON syntax, not Lucene syntax. Ex: " +
- "{\n" +
- "\t\"query\": {\n" +
- "\t\t\"match\": {\n" +
- "\t\t\t\"name\": \"John Smith\"\n" +
- "\t\t}\n" +
- "\t}\n" +
- "}")
+ .description("A query in JSON syntax, not Lucene syntax. Ex: {\"query\":{\"match\":{\"somefield\":\"somevalue\"}}}. " +
+ "If this parameter is not set, the query will be ready from the flowfile content.")
--- End diff --
Typo : 'read'
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180870941
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
+ .required(true)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor TYPE = new PropertyDescriptor.Builder()
+ .name("el-rest-type")
+ .displayName("Type")
+ .description("The type of this document (used by Elasticsearch for indexing and searching)")
+ .required(false)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor QUERY = new PropertyDescriptor.Builder()
+ .name("el-rest-query")
+ .displayName("Query")
+ .description("A query in JSON syntax, not Lucene syntax. Ex: " +
+ "{\n" +
--- End diff --
Done.
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r184432400
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/DeleteByQueryElasticsearch.java ---
@@ -0,0 +1,145 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.annotation.lifecycle.OnScheduled;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.elasticsearch.DeleteOperationResponse;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.AbstractProcessor;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_ALLOWED)
+@CapabilityDescription("Delete from an ElasticSearch index using a query. The query can be loaded from a flowfile body " +
+ "or from the Query parameter.")
+@Tags({ "elastic", "elasticsearch", "delete", "query"})
+@WritesAttributes({
+ @WritesAttribute(attribute = "elasticsearch.delete.took"),
--- End diff --
These should have descriptions as they will be displayed in the processor documentation
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2616#discussion_r180870705
--- Diff: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticSearchRestProcessor.java ---
@@ -0,0 +1,97 @@
+/*
+ * 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.elasticsearch;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.elasticsearch.ElasticSearchClientService;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+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;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+public interface ElasticSearchRestProcessor {
+ PropertyDescriptor INDEX = new PropertyDescriptor.Builder()
+ .name("el-rest-fetch-index")
+ .displayName("Index")
+ .description("The name of the index to read from")
+ .required(true)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor TYPE = new PropertyDescriptor.Builder()
+ .name("el-rest-type")
+ .displayName("Type")
+ .description("The type of this document (used by Elasticsearch for indexing and searching)")
+ .required(false)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .build();
+
+ PropertyDescriptor QUERY = new PropertyDescriptor.Builder()
+ .name("el-rest-query")
+ .displayName("Query")
+ .description("A query in JSON syntax, not Lucene syntax. Ex: " +
--- End diff --
Done
---
[GitHub] nifi pull request #2616: NIFI-5052 Added DeleteByQuery ElasticSearch process...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2616
---