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/06/06 11:35:11 UTC
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
GitHub user MikeThomsen opened a pull request:
https://github.com/apache/nifi/pull/2765
NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
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-5271
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2765.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 #2765
----
commit 514f4fc2793bba271056b5cd1c8a94dfde45247f
Author: Mike Thomsen <mi...@...>
Date: 2018-06-06T11:29:01Z
NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
----
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193611711
--- Diff: nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-services-api/pom.xml ---
@@ -45,6 +45,11 @@
<groupId>com.github.stephenc.findbugs</groupId>
<artifactId>findbugs-annotations</artifactId>
<version>1.3.9-1</version>
- </dependency>
+ </dependency>
--- End diff --
@MikeThomsen Can you please add a `To-do` here explaining that it is an workaround because of a transitive dependency issue. Ideally once the `google-auth-library-oauth2-http` is updated to a version that brings updated jackson artifacts, we can remove this.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
Link to my Gist that has the full log: https://gist.github.com/zenfenan/2e260004ce3d27ded2312a2f36d8b300
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2765
ran a full clean build w/contrib check prior to the fix for moving ObjectMapper creation inside the validator creation. If that all checks out via Travis-CI then I am a +1.
The L&N is now fine since Jackson is already properly accounted for in the three impacted Nars so no further L&N changes are needed.
@MikeThomsen and @zenfenan please be very careful in all changes such that anytime a dependency is added, removed, updated in terms of version that all impacted Nars and all assocated L&N impacts are considered. In the previous approach every single Nar that uses nifi-utils for StandardValidators would require the update and not only the L&N impact but also the size impact of having jackson libs in each but also the dependency heft of it in general.
We must be super dilligent on handling L&N so in general if you're altering dependencies or reviewing PRs that alter dependencies keep in mind we must account for every dependency be they direct or transitive in the appropriate L&N. It is a lot of work.
Thanks
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
Yes. I’m on windows. I built it with contrib-check. It built successfully
but when I execute run-nifi.bat, I got errors thrown during when the GCP
bundle gets loaded. I’m away from my machine now (on Phone). The error
stacktrace had messages along the lines NoClassDef : ObjectMapper. on
FasterXML. I’ll try to post the full log but extremely sorry for not able
to post them now.
>
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2765
@joewitt @zenfenan Made the change to a static version and added the requested TODO.
@zenfenan per our discussion about merges, I forgot to mention you need to make a squash commit out of the branch before doing the `--signoff`.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
The snippet of the log that throws the error when the service is starting:
```
2018-06-06 20:43:52,727 INFO [main] org.apache.nifi.web.server.JettyServer Loading WAR: E:\Workspace\ASF\nifi\nifi-assembly\target\nifi-1.7.0-SNAPSHOT-bin\nifi-1.7.0-SNAPSHOT\.\work\nar\framework\nifi-framework-nar-1.7.0-SNAPSHOT.nar-unpacked\META-INF\bundled-dependencies\nifi-web-docs-1.7.0-SNAPSHOT.war with context path set to /nifi-docs
2018-06-06 20:43:52,742 INFO [main] org.apache.nifi.web.server.JettyServer Loading documents web app with context path set to /nifi-docs
2018-06-06 20:43:52,742 INFO [main] org.apache.nifi.web.server.JettyServer Loading WAR: E:\Workspace\ASF\nifi\nifi-assembly\target\nifi-1.7.0-SNAPSHOT-bin\nifi-1.7.0-SNAPSHOT\.\work\nar\framework\nifi-framework-nar-1.7.0-SNAPSHOT.nar-unpacked\META-INF\bundled-dependencies\nifi-web-error-1.7.0-SNAPSHOT.war with context path set to /
2018-06-06 20:43:52,758 INFO [main] org.apache.nifi.web.server.JettyServer Running in HTTP mode; host headers not restricted
2018-06-06 20:43:54,747 ERROR [main] org.apache.nifi.NiFi Failure to launch NiFi due to java.util.ServiceConfigurationError: org.apache.nifi.controller.ControllerService: Provider org.apache.nifi.processors.gcp.credentials.service.GCPCredentialsControllerService could not be instantiated
java.util.ServiceConfigurationError: org.apache.nifi.controller.ControllerService: Provider org.apache.nifi.processors.gcp.credentials.service.GCPCredentialsControllerService could not be instantiated
at java.util.ServiceLoader.fail(ServiceLoader.java:232)
at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
at org.apache.nifi.nar.ExtensionManager.loadExtensions(ExtensionManager.java:148)
at org.apache.nifi.nar.ExtensionManager.discoverExtensions(ExtensionManager.java:123)
at org.apache.nifi.web.server.JettyServer.start(JettyServer.java:816)
at org.apache.nifi.NiFi.<init>(NiFi.java:157)
at org.apache.nifi.NiFi.<init>(NiFi.java:71)
at org.apache.nifi.NiFi.main(NiFi.java:292)
Caused by: java.lang.NoSuchMethodError: com.fasterxml.jackson.core.JsonFactory.requiresPropertyOrdering()Z
at com.fasterxml.jackson.databind.ObjectMapper.<init>(ObjectMapper.java:571)
at com.fasterxml.jackson.databind.ObjectMapper.<init>(ObjectMapper.java:480)
at org.apache.nifi.processor.util.JsonValidator.<init>(JsonValidator.java:29)
at org.apache.nifi.processors.gcp.credentials.factory.CredentialPropertyDescriptors.<clinit>(CredentialPropertyDescriptors.java:86)
at org.apache.nifi.processors.gcp.credentials.service.GCPCredentialsControllerService.<clinit>(GCPCredentialsControllerService.java:60)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at java.lang.Class.newInstance(Class.java:442)
at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
... 8 common frames omitted
2018-06-06 20:43:54,747 INFO [Thread-1] org.apache.nifi.NiFi Initiating shutdown of Jetty web server...
2018-06-06 20:43:54,747 INFO [Thread-1] org.apache.nifi.NiFi Jetty web server shutdown completed (nicely or otherwise).
```
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
@joewitt @MikeThomsen Thank you so much. I have merged it to the master +1
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193732582
--- Diff: nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-services-api/pom.xml ---
@@ -45,6 +45,11 @@
<groupId>com.github.stephenc.findbugs</groupId>
<artifactId>findbugs-annotations</artifactId>
<version>1.3.9-1</version>
- </dependency>
+ </dependency>
--- End diff --
Done.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
@joewitt @MikeThomsen
Build is a success but when I run (i.e. execute nifi.sh/run-nifi.bat), it
fails to bring up the service. That’s to do with the dependency issue in
GCP bundle. Can you please confirm you bring up the UI after these changes?
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193487112
--- Diff: nifi-commons/nifi-json-utils/src/main/java/org/apache/nifi/processor/util/JsonValidator.java ---
@@ -0,0 +1,48 @@
+/*
+ * 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.processor.util;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+import java.util.List;
+import java.util.Map;
+
+public class JsonValidator implements Validator {
+ private ObjectMapper mapper = new ObjectMapper();
--- End diff --
Ok. L&N feedback?
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2765
@zenfenan let me know if that works for you.
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193425267
--- Diff: nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/pom.xml ---
@@ -80,6 +80,12 @@
<artifactId>json</artifactId>
<version>1.8</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-json-utils</artifactId>
+ <version>1.7.0-SNAPSHOT</version>
--- End diff --
This causes issue when starting NiFi. Reason seems to be `google-auth-library-oauth2-http` comes with `jackson-core:2.1.3` as its dependency. We can't exclude it because of the version mismatch. Even the latest version of `google-auth-library-oauth2-http` uses `jackson-core:2.1.3`
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2765
@MikeThomsen i'm +1 go for it thanks
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2765
@MikeThomsen now that objectmapper is within the call any reason not to have this be a static method invoked consistently with other validators? I dont know that it matters but just a heads up.
As far as L&N i'll send more on that in a sec. It looks now that it will be very easy
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193427227
--- Diff: nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/pom.xml ---
@@ -80,6 +80,12 @@
<artifactId>json</artifactId>
<version>1.8</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-json-utils</artifactId>
+ <version>1.7.0-SNAPSHOT</version>
+ <scope>compile</scope>
--- End diff --
This is the one what I'm talking about.
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2765
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
Having it as static method sounds good to me.
@MikeThomsen Have you looked into the dependency issue with the GCP bundle
that I had raised earlier?
On Wed, 6 Jun 2018 at 10:43 PM, Joe Witt <no...@github.com> wrote:
> @MikeThomsen <https://github.com/MikeThomsen> now that objectmapper is
> within the call any reason not to have this be a static method invoked
> consistently with other validators? I dont know that it matters but just a
> heads up.
>
> As far as L&N i'll send more on that in a sec. It looks now that it will
> be very easy
>
> —
> You are receiving this because you were mentioned.
>
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/nifi/pull/2765#issuecomment-395144977>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AWmHLVvaj6GsMtK0fQtC6u8AfZjvtEB_ks5t6A2dgaJpZM4Uce2b>
> .
>
---
[GitHub] nifi pull request #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven mod...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2765#discussion_r193484189
--- Diff: nifi-commons/nifi-json-utils/src/main/java/org/apache/nifi/processor/util/JsonValidator.java ---
@@ -0,0 +1,48 @@
+/*
+ * 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.processor.util;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+
+import java.util.List;
+import java.util.Map;
+
+public class JsonValidator implements Validator {
+ private ObjectMapper mapper = new ObjectMapper();
--- End diff --
having this here means it is subject to use by multiple threads and reused. According to
https://github.com/FasterXML/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java#L78
We dont have guaranteed thread safety until the jackson 3.0 release.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2765
According to your gist, it cannot find that one method. Running dependency:tree on the GCP bundle it looks like the API module needs to have forcibly upgraded to 2.9.X due to a transitive dependency on 2.1.3. I'll tinker with that in a bit.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2765
@zenfenan @mattyb149 here you go.
@joewitt not sure what to do about adding license and notice files to this build. Can you review and advise?
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by zenfenan <gi...@git.apache.org>.
Github user zenfenan commented on the issue:
https://github.com/apache/nifi/pull/2765
@MikeThomsen We get compilation failure. Take a look at the travis logs.
---
[GitHub] nifi issue #2765: NIFI-5271 Moved JSON_VALIDATOR to its own maven module.
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2765
@zenfenan please share whatever logs you have/results you see so we can better understand. You're running on windows i take it?
Thanks
---