[PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 opened a new pull request, #8249: URL: https://github.com/apache/nifi/pull/8249 … # Summary [NIFI-12554](https://issues.apache.org/jira/browse/NIFI-12554) # Tracking Please complete the following tracking steps prior to pull request creation. ### Issue Tracking - [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created ### Pull Request Tracking - [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-0` - [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-0` ### Pull Request Formatting - [ ] Pull Request based on current revision of the `main` branch - [ ] 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 - [ ] Build completed using `mvn clean install -P contrib-check` - [ ] JDK 21 ### 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
exceptionfactory commented on code in PR #8249:
URL: https://github.com/apache/nifi/pull/8249#discussion_r1452770561
##
nifi-nar-bundles/nifi-jolt-bundle/pom.xml:
##
@@ -57,6 +45,22 @@
json-utils
${jolt.version}
+
+javax.servlet.jsp
+javax.servlet.jsp-api
+2.3.3
+
+
+javax.ws.rs
+javax.ws.rs-api
+2.1.1
+
Review Comment:
These dependencies are the old versions that are no longer current following
the upgrade to Spring 6 and Jetty 12. These should be replaced with the new
Jakarta equivalent versions.
##
nifi-nar-bundles/nifi-standard-bundle/pom.xml:
##
@@ -101,7 +99,7 @@
com.hierynomus
sshj
-0.38.0
+0.37.0
Review Comment:
This change should be reverted.
##
nifi-nar-bundles/nifi-standard-bundle/pom.xml:
##
@@ -210,6 +208,33 @@
json-flattener
0.16.6
+
+org.apache.bval
+bval-jsr
+1.1.2
+
+
+org.apache.tomcat
+tomcat-el-api
+
+
+org.apache.geronimo.specs
+geronimo-annotation_1.2_spec
+
+
+org.apache.geronimo.specs
+geronimo-jcdi_1.1.spec
+
+
+org.apache.geronimo.specs
+geronimo-jpa_2.0.spec
+
+
+org.apache.bval
+bval-xstream
+
+
+
Review Comment:
This dependency should be removed as it is not used.
##
nifi-nar-bundles/nifi-standard-bundle/pom.xml:
##
@@ -80,18 +77,19 @@
2.0.0-SNAPSHOT
-com.bazaarvoice.jolt
-jolt-core
-${jolt.version}
+javax.servlet.jsp
+javax.servlet.jsp-api
+2.3.3
-com.bazaarvoice.jolt
-json-utils
-${jolt.version}
+javax.servlet
+javax.servlet-api
+3.1.0
-jakarta.ws.rs
-jakarta.ws.rs-api
+javax.ws.rs
+javax.ws.rs-api
+2.1.1
Review Comment:
These javax dependencies are no longer current and should be removed.
##
nifi-nar-bundles/nifi-standard-bundle/pom.xml:
##
@@ -265,7 +290,7 @@
com.networknt
json-schema-validator
-1.1.0
+1.0.87
Review Comment:
This change should be reverted.
##
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-nar/src/main/resources/META-INF/NOTICE:
##
@@ -10,7 +10,7 @@ This product includes the following work from the Apache
Hadoop project under Ap
This includes derived works from the Apache Software License V2 library Jolt
(https://github.com/bazaarvoice/jolt)
Copyright 2013-2014 Bazaarvoice, Inc
The derived work is adapted from
com.bazaarvoice.jolt.chainr.ChainrBuilder.java,
com.bazaarvoice.jolt.chainr.spec.ChainrSpec.java,
- com.bazaarvoice.jolt.chainr.spec.ChainrEntry.java and can be found in the
org.apache.nifi.processors.standard.util.jolt.TransformFactory.java class.
+ com.bazaarvoice.jolt.chainr.spec.ChainrEntry.java and can be found in the
util.org.apache.nifi.processors.jolt.TransformFactory.java class.
Review Comment:
This appears to be incorrect and should be reverted.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on code in PR #8249: URL: https://github.com/apache/nifi/pull/8249#discussion_r1452773110 ## nifi-nar-bundles/nifi-standard-bundle/pom.xml: ## @@ -265,7 +290,7 @@ com.networknt json-schema-validator -1.1.0 +1.0.87 Review Comment: Kind of weird as I updated main and then on my branch I rebased with main, I am not sure why all these changes were not picked up. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on code in PR #8249: URL: https://github.com/apache/nifi/pull/8249#discussion_r1452774911 ## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-nar/src/main/resources/META-INF/NOTICE: ## @@ -10,7 +10,7 @@ This product includes the following work from the Apache Hadoop project under Ap This includes derived works from the Apache Software License V2 library Jolt (https://github.com/bazaarvoice/jolt) Copyright 2013-2014 Bazaarvoice, Inc The derived work is adapted from com.bazaarvoice.jolt.chainr.ChainrBuilder.java, com.bazaarvoice.jolt.chainr.spec.ChainrSpec.java, - com.bazaarvoice.jolt.chainr.spec.ChainrEntry.java and can be found in the org.apache.nifi.processors.standard.util.jolt.TransformFactory.java class. + com.bazaarvoice.jolt.chainr.spec.ChainrEntry.java and can be found in the util.org.apache.nifi.processors.jolt.TransformFactory.java class. Review Comment: Yeah I noticed that also after I pushed. You beat me to it :) -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
EndzeitBegins commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1893105650 Thank you for working on this @dan-s1. 👍🏻 I haven't done a full code review so I might've just overlooked something. But I noticed that the package names of the processors change due to the move from the standard-nar to the new jolt-nar. From my understanding this will result in the processors needing to be replaced manually on the canvas between version updates, or is there a migration in place for this sort of change? If not, and the user needs to replace those processors themself, I don't think we need to migrate properties as there won't be any instance of the processor existing anyway. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1894290354 @exceptionfactory Thanks for clarifying what the builds were failing on as I could not see that. I only saw 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1894548409 @exceptionfactory Do I need to add the new modules I created to `nifi-code-coverage` `pom.xml`? Which ones do I add `nifi-jolt-processors` and `nifi-jolt-utils` ? -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
exceptionfactory commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1894557055 > @exceptionfactory Do I need to add the new modules I created to `nifi-code-coverage` `pom.xml`? Which ones do I add `nifi-jolt-processors` and `nifi-jolt-utils` ? Yes, those two should be added and the old one should be removed. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1894633928 @exceptionfactory I am now able to see the stacktraces from the builds. Apparently there was also an issue with the `nifi-standard-utils` which no longer exists. So I removed both `nifi-standard-utils` and `nifi-jolt-record-processors` and replaced them with `nifi-jolt-processors` and `nifi-jolt-utils`. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1896124104 > Thank you for working on this @dan-s1. 👍🏻 > > I haven't done a full code review so I might've just overlooked something. But I noticed that the package names of the processors change due to the move from the standard-nar to the new jolt-nar. From my understanding this will result in the processors needing to be replaced manually on the canvas between version updates, or is there a migration in place for this sort of change? If not, and the user needs to replace those processors themself, I don't think we need to migrate properties as there won't be any instance of the processor existing anyway. @EndzeitBegins I am not sure the answer to your question. I was following what @exceptionfactory had told me as seen in [NIFI-12554](https://issues.apache.org/jira/browse/NIFI-12554) in the comment on 1/9/2024. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
exceptionfactory commented on code in PR #8249:
URL: https://github.com/apache/nifi/pull/8249#discussion_r1454379609
##
nifi-nar-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/AbstractJoltTransform.java:
##
@@ -0,0 +1,255 @@
+/*
+ * 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.jolt;
+
+import com.bazaarvoice.jolt.JoltTransform;
+import com.bazaarvoice.jolt.JsonUtils;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import org.apache.nifi.annotation.lifecycle.OnScheduled;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.PropertyValue;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.resource.ResourceCardinality;
+import org.apache.nifi.components.resource.ResourceReference;
+import org.apache.nifi.components.resource.ResourceType;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.jolt.util.JoltTransformStrategy;
+import org.apache.nifi.jolt.util.TransformFactory;
+import org.apache.nifi.processor.AbstractProcessor;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.util.StandardValidators;
+import org.apache.nifi.util.StringUtils;
+import org.apache.nifi.util.file.classloader.ClassLoaderUtils;
+
+import java.io.BufferedReader;
+import java.io.FilenameFilter;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public abstract class AbstractJoltTransform extends AbstractProcessor {
+public static final PropertyDescriptor JOLT_TRANSFORM = new
PropertyDescriptor.Builder()
+.name("Jolt Transform")
+.description("Specifies the Jolt Transformation that should be
used with the provided specification.")
+.required(true)
+.allowableValues(JoltTransformStrategy.class)
+.defaultValue(JoltTransformStrategy.CHAINR.getValue())
+.build();
+
+public static final PropertyDescriptor JOLT_SPEC = new
PropertyDescriptor.Builder()
+.name("Jolt Specification")
+.description("Jolt Specification for transformation of JSON data.
The value for this property may be the text of a Jolt specification "
++ "or the path to a file containing a Jolt specification.
'Jolt Specification' must be set, or "
++ "the value is ignored if the Jolt Sort Transformation is
selected.")
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+.identifiesExternalResource(ResourceCardinality.SINGLE,
ResourceType.FILE, ResourceType.TEXT)
+.required(false)
+.build();
+
+public static final PropertyDescriptor CUSTOM_CLASS = new
PropertyDescriptor.Builder()
+.name("Custom Transformation Class Name")
+.description("Fully Qualified Class Name for Custom
Transformation")
+.required(false)
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+.dependsOn(JOLT_TRANSFORM, JoltTransformStrategy.CUSTOMR)
+.build();
+
+public static final PropertyDescriptor MODULES = new
PropertyDescriptor.Builder()
+.name("Custom Module Directory")
+.description("Comma-separated list of paths to files and/or
directories which contain modules containing custom transformations (that are
not included on NiFi's classpath).")
+.required(false)
+.identifiesExternalResource(ResourceCardinality.MULTIPLE,
ResourceType.FILE, ResourceType.DIRECTORY)
+.expressionLanguageSupported(ExpressionLanguageScope.EN
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
dan-s1 commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1900735102 > nifi-standard-shared-nar > @dan-s1 in the process of testing, it is clear that the migrateProperties method is not useful in this scenario. I wasn't quite clear on the initial comment, but that method can be removed. With moving these components to a new NAR, they are effectively treated as new components, so automated migration is not possible. > > The only other high-level change is to have the `nifi-jolt-bundle` use `nifi-standard-shared-bom` as the parent, and set `nifi-standard-shared-nar` as the NAR dependency in `nifi-jolt-nar` so that the Jolt NAR does not include additional copies of the Jackson JAR libraries. @exceptionfactory I thought the `nifi-standard-shared-nar` is already a dependency in the `nifi-jolt-nar`. Below is a cut and paste from the `nifi-jolt-nar` `pom.xml` ``` org.apache.nifi nifi-standard-shared-nar nar ``` -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
exceptionfactory commented on PR #8249: URL: https://github.com/apache/nifi/pull/8249#issuecomment-1900749975 > > nifi-standard-shared-nar > > > @dan-s1 in the process of testing, it is clear that the migrateProperties method is not useful in this scenario. I wasn't quite clear on the initial comment, but that method can be removed. With moving these components to a new NAR, they are effectively treated as new components, so automated migration is not possible. > > The only other high-level change is to have the `nifi-jolt-bundle` use `nifi-standard-shared-bom` as the parent, and set `nifi-standard-shared-nar` as the NAR dependency in `nifi-jolt-nar` so that the Jolt NAR does not include additional copies of the Jackson JAR libraries. > > @exceptionfactory I thought the `nifi-standard-shared-nar` is already a dependency in the `nifi-jolt-nar`. Below is a cut and paste from the `nifi-jolt-nar` `pom.xml` > > ``` > > org.apache.nifi > nifi-standard-shared-nar > nar > > ``` Thanks @dan-s1, you are correct, I just noted that for completeness. The other element still needs to be changed so that the bundle uses `nifi-standard-shared-bom` as the parent, which will mark the appropriate dependencies as provided, thus reducing the NAR size. If you could address that, and rebase on the current main branch, that would be helpful. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. [nifi]
exceptionfactory closed pull request #8249: NIFI-12554 Placed all Jolt related code under a new nifi-jolt-bundle and refactored to reduce duplicate code. URL: https://github.com/apache/nifi/pull/8249 -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
