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/11/07 21:01:49 UTC
[GitHub] [nifi] ChrisSamo632 opened a new pull request, #6628: NIFI-10067 enable use of script for Elasticsearch updates
ChrisSamo632 opened a new pull request, #6628:
URL: https://github.com/apache/nifi/pull/6628
# Summary
[NIFI-10067](https://issues.apache.org/jira/browse/NIFI-10067) enable use of script when updating documents in Elasticsearch via PutElasticsearchJson or PutElasticsearchRecord
[NIFI-3263](https://issues.apache.org/jira/browse/NIFI-3263) enable use of dynamic_templates in Elasticsearch _bulk operations
NIFI-3263 enable use of bulk header fields in Elasticsearch _bulk operations via BULK: dynamic headers in PutElasticsearchJson and PutElasticsearchRecord
# 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
- [ ] JDK 11
- [x] 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] MikeThomsen commented on pull request #6628: NIFI-10067 enable use of script for Elasticsearch updates
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1308972782
> Or have you tried it and found something not working/you think could be improved?
My vision for that processor would be a query builder that allows you to break down the process. Ex specifying the script or script id and then the update query. That combined with the ability to just use the flowfile body as a raw document to punt to update_by_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 pull request #6628: NIFI-10067 enable use of script for Elasticsearch updates
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1307979059
Are you planning to update the `UpdateByQueryElasticsearch` processor to support scripts? I was planning to do that and didn't see it getting updated in your PR.
--
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 #6628: NIFI-10067 enable use of script for Elasticsearch updates
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1308273945
> Are you planning to update the `UpdateByQueryElasticsearch` processor to support scripts? I was planning to do that and didn't see it getting updated in your PR.
I don't believe there's anything that needs doing - the processor accepts arbitrary json as the "query" and sends that to elasticsearch, so you can already provide a `script` object within that
Or have you tried it and found something not working/you think could be improved?
Hopefully all that's left for this PR now is a rebase and finishing the additional docs for the PUT processors to describe the new user of dynamic properties to populate the bulk api request headers
--
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 #6628: NIFI-10067 enable use of script, dynamic_templates and _bulk headers for PutElasticsearchJson/Record processors
Posted by "ChrisSamo632 (via GitHub)" <gi...@apache.org>.
ChrisSamo632 commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1492070207
`scripted_upsert` option added to the `PutElasticsearchJson` and `PutElasticsearchRecord` processors (with associated Integration Tests in the `ElasticsearchClientServiceImpl`
--
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 #6628: NIFI-10067 enable use of script, dynamic_templates and _bulk headers for PutElasticsearchJson/Record processors
Posted by "ChrisSamo632 (via GitHub)" <gi...@apache.org>.
ChrisSamo632 commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1491468884
A community member has [asked on Slack](https://apachenifi.slack.com/archives/C0L9VCD47/p1680008964668749?thread_ts=1680008108.684529&cid=C0L9VCD47) whether this change can also incorporate the option for [scripted_upsert](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-update.html#scripted_upsert).
This would already be possible through the `PutElasticsearchJson` processor as it simply sends the FlowFile content as the request body to Elasticsearch, so the NiFi Flow just needs to ensure there's the `scripted_upsert: true` option set within the FlwoFile content before calling the `PutElasticsearchJson` processor.
An option to set this flag for the `PutElasticsearchRecord` processor should be added as part of this PR (it would make sense to do so, should be simple and hopefully completes the current set of Elasticsearch Update API options available to clients).
--
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 #6628: NIFI-10067 enable use of script for Elasticsearch updates
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1308978863
> > Or have you tried it and found something not working/you think could be improved?
>
> My vision for that processor would be a query builder that allows you to break down the process. Ex specifying the script or script id and then the update query. That combined with the ability to just use the flowfile body as a raw document to punt to update_by_query.
That (to me) sounds like a (potentially breaking) change to the processor's API, best left to another ticket/discussion
I'm happy to update the existing processor's documentation to clarify that the `query` property for `UpdateByQuery` is currently the request body that's sent to Elasticsearch for the `_update_by_query` endpoint, i.e. if you want to run a Script, write your JSON like `{"script": {"source": "..."}, "query": {"match_all":{}}}`
--
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 #6628: NIFI-10067 enable use of script, dynamic_templates and _bulk headers for PutElasticsearchJson/Record processors
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #6628: NIFI-10067 enable use of script, dynamic_templates and _bulk headers for PutElasticsearchJson/Record processors
URL: https://github.com/apache/nifi/pull/6628
--
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 #6628: NIFI-10067 enable use of script for Elasticsearch updates
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on PR #6628:
URL: https://github.com/apache/nifi/pull/6628#issuecomment-1367243167
> My vision for that processor...
@MikeThomsen FYI I raised [NIFI-11016](https://issues.apache.org/jira/browse/NIFI-11016) to cover your future update to the `UpdateByQueryElasticsearch` processor if you wanted to do that in future
--
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] gresockj commented on a diff in pull request #6628: NIFI-10067 enable use of script, dynamic_templates and _bulk headers for PutElasticsearchJson/Record processors
Posted by "gresockj (via GitHub)" <gi...@apache.org>.
gresockj commented on code in PR #6628:
URL: https://github.com/apache/nifi/pull/6628#discussion_r1103862669
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java:
##########
@@ -190,6 +201,26 @@ public class PutElasticsearchRecord extends AbstractPutElasticsearch {
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.build();
+ static final PropertyDescriptor SCRIPT_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-es-record-script-path")
+ .displayName("script Record Path")
Review Comment:
Perhaps capital S
##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchJson.java:
##########
@@ -87,6 +98,25 @@ public class PutElasticsearchJson extends AbstractPutElasticsearch {
.addValidator(StandardValidators.ATTRIBUTE_KEY_VALIDATOR)
.build();
+ static final PropertyDescriptor SCRIPT = new PropertyDescriptor.Builder()
+ .name("put-es-json-script")
+ .displayName("Script")
+ .description("The script for the document update/upsert. Only applies to Update/Upsert operations. " +
+ "Must be parsable as JSON Object. " +
+ "If left blank, the FlowFile content will be used for document update/upsert")
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .build();
+
+ static final PropertyDescriptor DYNAMIC_TEMPLATES = new PropertyDescriptor.Builder()
+ .name("put-es-json-dynamic_templates")
+ .displayName("Script")
Review Comment:
I think this is a copy/paste error
--
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