You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/10/17 17:17:05 UTC
[GitHub] [nifi] ChrisSamo632 opened a new pull request, #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
ChrisSamo632 opened a new pull request, #6544:
URL: https://github.com/apache/nifi/pull/6544
<!-- 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. -->
# Summary
[NIFI-9398](https://issues.apache.org/jira/browse/NIFI-9398) add verification for the Elasticsearch REST API processors and controller services.
Additionally:
- add `elasticsearch8` tags to Elasticsearch REST API processors
- address various compiler warnings (i.e. from IntelliJ) in associated classes
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [x] Pull Request based on current revision of the `main` branch
- [x] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [x] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [x] JDK 11
- [ ] JDK 17
### Licensing
- ~[ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)~
- ~[ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files~
### Documentation
- ~[ ] Documentation formatting appears as expected in rendered files~
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014600503
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientServiceImpl.java:
##########
@@ -416,6 +495,26 @@ public void refresh(final String index, final Map<String, String> requestParamet
}
}
+ @Override
+ public boolean exists(final String index, final Map<String, String> requestParameters) {
+ try {
+ final Response response = performRequest("HEAD", "/" + index, requestParameters, null);
Review Comment:
I think this is a common thing throughout the methods in this class and was wondering the same myself (but then forgot) - I'll make a general change to ensure that a leading `/` is always present but that we don't end up with duplicates (e.g. `host:port//index`)
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014601005
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticsearchRestProcessor.java:
##########
@@ -117,13 +123,66 @@ default String getQuery(final FlowFile input, final ProcessContext context, fina
}
default Map<String, String> getUrlQueryParameters(final ProcessContext context, final FlowFile flowFile) {
+ return getUrlQueryParameters(context, flowFile != null ? flowFile.getAttributes() : null);
+ }
+
+ default Map<String, String> getUrlQueryParameters(final ProcessContext context, final Map<String, String> attributes) {
return context.getProperties().entrySet().stream()
// filter non-null dynamic properties
.filter(e -> e.getKey().isDynamic() && e.getValue() != null)
// convert to Map of URL parameter keys and values
.collect(Collectors.toMap(
- e -> e.getKey().getName(),
- e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(flowFile).getValue()
+ e -> e.getKey().getName(),
+ e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(attributes).getValue()
));
}
+
+ String VERIFICATION_STEP_INDEX_EXISTS = "Elasticsearch Index Exists";
+
+ @Override
+ default List<ConfigVerificationResult> verify(final ProcessContext context, final ComponentLog verificationLogger, final Map<String, String> attributes) {
Review Comment:
Sounds like you might have been confused by the NiFi `verify` process as I was the first time I reviewed this sort of thing (I think there was a documentation Jira raised off the back of a Slack conversation I started, but not sure it's been done yet)
Verification in NiFi is an optional thing for users to test their processor/controller settings before enabling/starting their components to verify whether they've got things like connection details setup correctly. Here, I've set the "Index Exists Check" to `SUCCEED` whether the index exists or not, provided the connection to Elasticsearch works and the expected not/exists response is returned. So it's OK if the index doesn't exist (and I'd see that happening not just for templated indices but any index - the index could be created by the first use of `PutElasticsearchRecord`, for example)
Verification in NiFi is **not** mandatory and if a verify check fails, it doesn't stop the user from enabling/starting their components - but here, it should pass whether or not the index exists provided the connection to Elasticsearch succeeds and there are no authentication issues - the user is given the opportunity to provide attribute values via the verification window, e.g. if they're using NiFi Expression Language to specify an index name at runtime from FlowFile attributes, they can simulate that in the UI for verification
--
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
[GitHub] [nifi] ChrisSamo632 commented on pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6544:
URL: https://github.com/apache/nifi/pull/6544#issuecomment-1281196028
Marked as `Draft` until I've completed more manual verifications via the NiFi UI.
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014600354
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/GetElasticsearch.java:
##########
@@ -213,7 +273,7 @@ private void handleElasticsearchException(final ElasticsearchException ese, Flow
getLogger().error(msg, ese);
if (input != null) {
session.penalize(input);
- input = session.putAttribute(input, "elasticsearch.get.error", ese.getMessage());
+ session.putAttribute(input, "elasticsearch.get.error", ese.getMessage());
Review Comment:
I was thinking that `input` wasn't then passed back out of the method and so there was no need to re-assign it, but actually it's used in the subsequent `session.transfer`, so agree this should be re-instated
--
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
[GitHub] [nifi] MikeThomsen commented on pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on PR #6544:
URL: https://github.com/apache/nifi/pull/6544#issuecomment-1304551211
@ChrisSamo632 see feedback above. When I ran your branch on the command line, I got a bunch of NoClassDef errors because testcontainers has an explicit scope of `test` set in the root pom. `mvn dependency:tree` was showing that testcontainers was not coming through as a transitive dependency via the test utils jar.
--
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
[GitHub] [nifi] asfgit closed pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
URL: https://github.com/apache/nifi/pull/6544
--
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
[GitHub] [nifi] ChrisSamo632 commented on pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6544:
URL: https://github.com/apache/nifi/pull/6544#issuecomment-1304581690
> @ChrisSamo632 see feedback above. When I ran your branch on the command line, I got a bunch of NoClassDef errors because testcontainers has an explicit scope of `test` set in the root pom. `mvn dependency:tree` was showing that testcontainers was not coming through as a transitive dependency via the test utils jar.
@MikeThomsen thanks for the review, I think I've addressed all your comments now (I unresolved those in the discussion that I believed needed changes to be made, that way I/you can see what needs re-reviewing).
Also updated the 7.x and 8.x image versions to the latest available (new releases from Elastic recently).
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014633597
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticsearchRestProcessor.java:
##########
@@ -117,13 +123,66 @@ default String getQuery(final FlowFile input, final ProcessContext context, fina
}
default Map<String, String> getUrlQueryParameters(final ProcessContext context, final FlowFile flowFile) {
+ return getUrlQueryParameters(context, flowFile != null ? flowFile.getAttributes() : null);
+ }
+
+ default Map<String, String> getUrlQueryParameters(final ProcessContext context, final Map<String, String> attributes) {
return context.getProperties().entrySet().stream()
// filter non-null dynamic properties
.filter(e -> e.getKey().isDynamic() && e.getValue() != null)
// convert to Map of URL parameter keys and values
.collect(Collectors.toMap(
- e -> e.getKey().getName(),
- e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(flowFile).getValue()
+ e -> e.getKey().getName(),
+ e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(attributes).getValue()
));
}
+
+ String VERIFICATION_STEP_INDEX_EXISTS = "Elasticsearch Index Exists";
+
+ @Override
+ default List<ConfigVerificationResult> verify(final ProcessContext context, final ComponentLog verificationLogger, final Map<String, String> attributes) {
Review Comment:
To be fair, maybe it should be a `FAILED` status for processors that wouldn't create the index, e.g. Get or Query
--
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
[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014638767
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
Review Comment:
> The version is taken from the testcontainers in the top-level nifi/pom.xml's dependencyManagement for all subsequent child POMs using testcontainer imports
The root pom's `<dependencyManagement/>` section has an explicit scope of `test` set on testcontainers dependencies.
--
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
[GitHub] [nifi] ChrisSamo632 commented on pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6544:
URL: https://github.com/apache/nifi/pull/6544#issuecomment-1289476430
> Marked as `Draft` until I've completed more manual verifications via the NiFi UI.
Added integration-tests for `nifi-elasticsearch-restapi-processors` to cover some Processor Verification steps (shares Testcontainer setup with `nifi-elasticsearch-client-service` using a new common `nifi-elasticsearch-integration-test-utils` module)
--
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
[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1012410138
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientServiceImpl.java:
##########
@@ -416,6 +495,26 @@ public void refresh(final String index, final Map<String, String> requestParamet
}
}
+ @Override
+ public boolean exists(final String index, final Map<String, String> requestParameters) {
+ try {
+ final Response response = performRequest("HEAD", "/" + index, requestParameters, null);
Review Comment:
Should probably check to make sure that `index` has no leading or trailing `/`
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
Review Comment:
Should be made into a compile time dependency w/ a version number.
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticsearchRestProcessor.java:
##########
@@ -117,13 +123,66 @@ default String getQuery(final FlowFile input, final ProcessContext context, fina
}
default Map<String, String> getUrlQueryParameters(final ProcessContext context, final FlowFile flowFile) {
+ return getUrlQueryParameters(context, flowFile != null ? flowFile.getAttributes() : null);
+ }
+
+ default Map<String, String> getUrlQueryParameters(final ProcessContext context, final Map<String, String> attributes) {
return context.getProperties().entrySet().stream()
// filter non-null dynamic properties
.filter(e -> e.getKey().isDynamic() && e.getValue() != null)
// convert to Map of URL parameter keys and values
.collect(Collectors.toMap(
- e -> e.getKey().getName(),
- e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(flowFile).getValue()
+ e -> e.getKey().getName(),
+ e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(attributes).getValue()
));
}
+
+ String VERIFICATION_STEP_INDEX_EXISTS = "Elasticsearch Index Exists";
+
+ @Override
+ default List<ConfigVerificationResult> verify(final ProcessContext context, final ComponentLog verificationLogger, final Map<String, String> attributes) {
Review Comment:
I have reservations about this approach for verification because it's very possible users could end up using EL to specify a dynamic index name based on an Elastic template. How do you see that scenario playing out with verification since the index may not exist yet for a good reason? Also, this could potentially block users who are iterating quickly and relying on Elastic to autogenerate the schema during their initial setup before formalizing one.
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/GetElasticsearch.java:
##########
@@ -213,7 +273,7 @@ private void handleElasticsearchException(final ElasticsearchException ese, Flow
getLogger().error(msg, ese);
if (input != null) {
session.penalize(input);
- input = session.putAttribute(input, "elasticsearch.get.error", ese.getMessage());
+ session.putAttribute(input, "elasticsearch.get.error", ese.getMessage());
Review Comment:
Why did you drop the assignment?
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticsearchRestProcessor.java:
##########
@@ -117,13 +123,66 @@ default String getQuery(final FlowFile input, final ProcessContext context, fina
}
default Map<String, String> getUrlQueryParameters(final ProcessContext context, final FlowFile flowFile) {
+ return getUrlQueryParameters(context, flowFile != null ? flowFile.getAttributes() : null);
+ }
+
+ default Map<String, String> getUrlQueryParameters(final ProcessContext context, final Map<String, String> attributes) {
return context.getProperties().entrySet().stream()
// filter non-null dynamic properties
.filter(e -> e.getKey().isDynamic() && e.getValue() != null)
// convert to Map of URL parameter keys and values
.collect(Collectors.toMap(
- e -> e.getKey().getName(),
- e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(flowFile).getValue()
+ e -> e.getKey().getName(),
+ e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(attributes).getValue()
));
}
+
+ String VERIFICATION_STEP_INDEX_EXISTS = "Elasticsearch Index Exists";
+
+ @Override
+ default List<ConfigVerificationResult> verify(final ProcessContext context, final ComponentLog verificationLogger, final Map<String, String> attributes) {
+ final List<ConfigVerificationResult> results = new ArrayList<>();
+ final ConfigVerificationResult.Builder indexExistsResult = new ConfigVerificationResult.Builder()
+ .verificationStepName(VERIFICATION_STEP_INDEX_EXISTS);
+
+ ElasticSearchClientService verifyClientService = null;
+ String index = null;
+ boolean indexExists = false;
+ if (context.getProperty(CLIENT_SERVICE).isSet()) {
+ verifyClientService = context.getProperty(CLIENT_SERVICE).asControllerService(ElasticSearchClientService.class);
+ if (context.getProperty(INDEX).isSet()) {
+ index = context.getProperty(INDEX).evaluateAttributeExpressions(attributes).getValue();
+ try {
+ if (verifyClientService.exists(index, getUrlQueryParameters(context, attributes))) {
+ indexExistsResult.outcome(ConfigVerificationResult.Outcome.SUCCESSFUL)
+ .explanation(String.format("Index [%s] exists", index));
+ indexExists = true;
+ } else {
+ indexExistsResult.outcome(ConfigVerificationResult.Outcome.SUCCESSFUL)
Review Comment:
Related to the point above, I guess the goal here is to just flex the REST api and make sure you can get an expected response (even if it's an index doesn't exist exception)?
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
Review Comment:
Should be made into a compile time dependency w/ a version number.
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014646933
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
Review Comment:
Seemed to work for me, maybe I'd changed the scope and not rebuilt the main pom or something, no matter, will add the compile scope
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014601265
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
Review Comment:
Default `scope` in Maven is `compile`, so don't need to be explicit (although we can if wanted for all dependencies)
The `version` is taken from the `testcontainers` in the top-level `nifi/pom.xml`'s `dependencyManagement` for all subsequent child POMs using `testcontainer` imports
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014601052
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/ElasticsearchRestProcessor.java:
##########
@@ -117,13 +123,66 @@ default String getQuery(final FlowFile input, final ProcessContext context, fina
}
default Map<String, String> getUrlQueryParameters(final ProcessContext context, final FlowFile flowFile) {
+ return getUrlQueryParameters(context, flowFile != null ? flowFile.getAttributes() : null);
+ }
+
+ default Map<String, String> getUrlQueryParameters(final ProcessContext context, final Map<String, String> attributes) {
return context.getProperties().entrySet().stream()
// filter non-null dynamic properties
.filter(e -> e.getKey().isDynamic() && e.getValue() != null)
// convert to Map of URL parameter keys and values
.collect(Collectors.toMap(
- e -> e.getKey().getName(),
- e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(flowFile).getValue()
+ e -> e.getKey().getName(),
+ e -> context.getProperty(e.getKey()).evaluateAttributeExpressions(attributes).getValue()
));
}
+
+ String VERIFICATION_STEP_INDEX_EXISTS = "Elasticsearch Index Exists";
+
+ @Override
+ default List<ConfigVerificationResult> verify(final ProcessContext context, final ComponentLog verificationLogger, final Map<String, String> attributes) {
+ final List<ConfigVerificationResult> results = new ArrayList<>();
+ final ConfigVerificationResult.Builder indexExistsResult = new ConfigVerificationResult.Builder()
+ .verificationStepName(VERIFICATION_STEP_INDEX_EXISTS);
+
+ ElasticSearchClientService verifyClientService = null;
+ String index = null;
+ boolean indexExists = false;
+ if (context.getProperty(CLIENT_SERVICE).isSet()) {
+ verifyClientService = context.getProperty(CLIENT_SERVICE).asControllerService(ElasticSearchClientService.class);
+ if (context.getProperty(INDEX).isSet()) {
+ index = context.getProperty(INDEX).evaluateAttributeExpressions(attributes).getValue();
+ try {
+ if (verifyClientService.exists(index, getUrlQueryParameters(context, attributes))) {
+ indexExistsResult.outcome(ConfigVerificationResult.Outcome.SUCCESSFUL)
+ .explanation(String.format("Index [%s] exists", index));
+ indexExists = true;
+ } else {
+ indexExistsResult.outcome(ConfigVerificationResult.Outcome.SUCCESSFUL)
Review Comment:
Correct, I've tried to describe this further in https://github.com/apache/nifi/pull/6544#discussion_r1014601005
--
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
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6544: NIFI-9398 add verification to ElasticSearchClientService (with integration tests) and Elasticsearch REST API processors
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6544:
URL: https://github.com/apache/nifi/pull/6544#discussion_r1014601308
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-test-utils/pom.xml:
##########
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- 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. -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>nifi-elasticsearch-bundle</artifactId>
+ <groupId>org.apache.nifi</groupId>
+ <version>1.19.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>nifi-elasticsearch-test-utils</artifactId>
+ <packaging>jar</packaging>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-mock</artifactId>
+ <version>1.19.0-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.elasticsearch.client</groupId>
+ <artifactId>elasticsearch-rest-client</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>testcontainers</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>elasticsearch</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
Review Comment:
As per https://github.com/apache/nifi/pull/6544/files#r1014601265
--
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