[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user bbende commented on a diff in the pull request: https://github.com/apache/nifi/pull/3041#discussion_r228563247 --- Diff: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/pom.xml --- @@ -147,6 +147,12 @@ 2.2.1 test + +org.apache.nifi +nifi-solr-client-api +1.8.0-SNAPSHOT +compile --- End diff -- Was able to try it out and you are right that it is working, but currently this is only because the service impl happens to be in the same NAR as the processors. If someone implemented their own SolrClientService in another NAR then it wouldn't be able to be used in the current state. If you just change the above dependency to provided and then change nifi-solr-nar/pom.xml where it has: ``` org.apache.nifi nifi-standard-services-api-nar 1.8.0-SNAPSHOT nar ``` To ``` org.apache.nifi nifi-solr-client-api-nar 1.8.0-SNAPSHOT nar ``` That should make it work correctly for both cases. ---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user bbende commented on a diff in the pull request: https://github.com/apache/nifi/pull/3041#discussion_r228542809 --- Diff: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/pom.xml --- @@ -147,6 +147,12 @@ 2.2.1 test + +org.apache.nifi +nifi-solr-client-api +1.8.0-SNAPSHOT +compile --- End diff -- This should be provided scope, using compile scope would mean the nifi-solr-client-api JAR would be packaged into two different NARs (the API NAR and the processors NAR) and at runtime it should only be in the API NAR. This relates to the other comment that the nifi-solr-nar pom needs to be updated to change its parent from standard services to the nifi-solr-client-api-nar. I'm not in a state to try it right now, but I would suspect that running a full build of this branch you would be able to create a SolrClientService, but not assign it in one of the Solr processors. ---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user MikeThomsen commented on a diff in the pull request: https://github.com/apache/nifi/pull/3041#discussion_r228537197 --- Diff: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/pom.xml --- @@ -147,6 +147,12 @@ 2.2.1 test + +org.apache.nifi +nifi-solr-client-api +1.8.0-SNAPSHOT +provided --- End diff -- > The solr-bundle NAR pom also needs to be updated to have a parent of the client API NAR. Could you explain? I added the client api nar to the assemble, rebuilt and was able to bring in a new client service just fine. ---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3041#discussion_r228507330
--- Diff:
nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/main/java/org/apache/nifi/processors/solr/SolrProcessor.java
---
@@ -176,70 +172,11 @@ protected final KeytabUser getKerberosKeytabUser() {
final protected Collection
customValidate(ValidationContext context) {
final List problems = new ArrayList<>();
-if
(SOLR_TYPE_CLOUD.equals(context.getProperty(SOLR_TYPE).getValue())) {
-final String collection =
context.getProperty(COLLECTION).getValue();
-if (collection == null || collection.trim().isEmpty()) {
-problems.add(new ValidationResult.Builder()
-.subject(COLLECTION.getName())
-.input(collection).valid(false)
-.explanation("A collection must specified for Solr
Type of Cloud")
-.build());
-}
-}
-
-// For solr cloud the location will be the ZooKeeper host:port so
we can't validate the SSLContext, but for standard solr
-// we can validate if the url starts with https we need an
SSLContextService, if it starts with http we can't have an SSLContextService
-if
(SOLR_TYPE_STANDARD.equals(context.getProperty(SOLR_TYPE).getValue())) {
-final String solrLocation =
context.getProperty(SOLR_LOCATION).evaluateAttributeExpressions().getValue();
-if (solrLocation != null) {
-final SSLContextService sslContextService =
context.getProperty(SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
-if (solrLocation.startsWith("https:") && sslContextService
== null) {
-problems.add(new ValidationResult.Builder()
-.subject(SSL_CONTEXT_SERVICE.getDisplayName())
-.valid(false)
-.explanation("an SSLContextService must be
provided when using https")
-.build());
-} else if (solrLocation.startsWith("http:") &&
sslContextService != null) {
-problems.add(new ValidationResult.Builder()
-.subject(SSL_CONTEXT_SERVICE.getDisplayName())
-.valid(false)
-.explanation("an SSLContextService can not be
provided when using http")
-.build());
-}
-}
-}
-
-// Validate that we username and password are provided together,
or that neither are provided
-final String username =
context.getProperty(BASIC_USERNAME).evaluateAttributeExpressions().getValue();
-final String password =
context.getProperty(BASIC_PASSWORD).evaluateAttributeExpressions().getValue();
-
-final boolean basicUsernameProvided =
!StringUtils.isBlank(username);
-final boolean basicPasswordProvided =
!StringUtils.isBlank(password);
-
-if (basicUsernameProvided && !basicPasswordProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(BASIC_PASSWORD.getDisplayName())
-.valid(false)
-.explanation("a password must be provided for the
given username")
-.build());
-}
-
-if (basicPasswordProvided && !basicUsernameProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(BASIC_USERNAME.getDisplayName())
-.valid(false)
-.explanation("a username must be provided for the
given password")
-.build());
-}
-
-// Validate that only kerberos or basic auth can be set, but not
both
-final KerberosCredentialsService kerberosCredentialsService =
context.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
-if (kerberosCredentialsService != null && basicUsernameProvided &&
basicPasswordProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(KERBEROS_CREDENTIALS_SERVICE.getDisplayName())
-.valid(false)
-.explanation("basic auth and kerberos cannot be
configured at the same time")
-.build());
+List _temp = new
ArrayList<>(validateConnectionDetails(context));
+if (_temp.size() == 0 &&
context.getProperty(CLIENT_SERVICE).isSet()) {
--- End diff --
Fixed that too.
---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user bbende commented on a diff in the pull request: https://github.com/apache/nifi/pull/3041#discussion_r222379389 --- Diff: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/pom.xml --- @@ -147,6 +147,12 @@ 2.2.1 test + +org.apache.nifi +nifi-solr-client-api +1.8.0-SNAPSHOT +provided --- End diff -- The solr-bundle NAR pom also needs to be updated to have a parent of the client API NAR. Also, the assembly pom needs to include the client API NAR so it ends up in the final build. ---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3041#discussion_r222376374
--- Diff:
nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/main/java/org/apache/nifi/processors/solr/SolrProcessor.java
---
@@ -176,70 +172,11 @@ protected final KeytabUser getKerberosKeytabUser() {
final protected Collection
customValidate(ValidationContext context) {
final List problems = new ArrayList<>();
-if
(SOLR_TYPE_CLOUD.equals(context.getProperty(SOLR_TYPE).getValue())) {
-final String collection =
context.getProperty(COLLECTION).getValue();
-if (collection == null || collection.trim().isEmpty()) {
-problems.add(new ValidationResult.Builder()
-.subject(COLLECTION.getName())
-.input(collection).valid(false)
-.explanation("A collection must specified for Solr
Type of Cloud")
-.build());
-}
-}
-
-// For solr cloud the location will be the ZooKeeper host:port so
we can't validate the SSLContext, but for standard solr
-// we can validate if the url starts with https we need an
SSLContextService, if it starts with http we can't have an SSLContextService
-if
(SOLR_TYPE_STANDARD.equals(context.getProperty(SOLR_TYPE).getValue())) {
-final String solrLocation =
context.getProperty(SOLR_LOCATION).evaluateAttributeExpressions().getValue();
-if (solrLocation != null) {
-final SSLContextService sslContextService =
context.getProperty(SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
-if (solrLocation.startsWith("https:") && sslContextService
== null) {
-problems.add(new ValidationResult.Builder()
-.subject(SSL_CONTEXT_SERVICE.getDisplayName())
-.valid(false)
-.explanation("an SSLContextService must be
provided when using https")
-.build());
-} else if (solrLocation.startsWith("http:") &&
sslContextService != null) {
-problems.add(new ValidationResult.Builder()
-.subject(SSL_CONTEXT_SERVICE.getDisplayName())
-.valid(false)
-.explanation("an SSLContextService can not be
provided when using http")
-.build());
-}
-}
-}
-
-// Validate that we username and password are provided together,
or that neither are provided
-final String username =
context.getProperty(BASIC_USERNAME).evaluateAttributeExpressions().getValue();
-final String password =
context.getProperty(BASIC_PASSWORD).evaluateAttributeExpressions().getValue();
-
-final boolean basicUsernameProvided =
!StringUtils.isBlank(username);
-final boolean basicPasswordProvided =
!StringUtils.isBlank(password);
-
-if (basicUsernameProvided && !basicPasswordProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(BASIC_PASSWORD.getDisplayName())
-.valid(false)
-.explanation("a password must be provided for the
given username")
-.build());
-}
-
-if (basicPasswordProvided && !basicUsernameProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(BASIC_USERNAME.getDisplayName())
-.valid(false)
-.explanation("a username must be provided for the
given password")
-.build());
-}
-
-// Validate that only kerberos or basic auth can be set, but not
both
-final KerberosCredentialsService kerberosCredentialsService =
context.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
-if (kerberosCredentialsService != null && basicUsernameProvided &&
basicPasswordProvided) {
-problems.add(new ValidationResult.Builder()
-.subject(KERBEROS_CREDENTIALS_SERVICE.getDisplayName())
-.valid(false)
-.explanation("basic auth and kerberos cannot be
configured at the same time")
-.build());
+List _temp = new
ArrayList<>(validateConnectionDetails(context));
+if (_temp.size() == 0 &&
context.getProperty(CLIENT_SERVICE).isSet()) {
--- End diff --
The onScheduled method looks like it hasn't been changed and is not
check
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3041#discussion_r222377115
--- Diff: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-client-api/pom.xml ---
@@ -0,0 +1,68 @@
+
+
+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
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+4.0.0
+
+org.apache.nifi
+nifi-solr-bundle
+1.8.0-SNAPSHOT
+
+nifi-solr-client-api
+jar
+
+6.2.0
+
+
+
+org.apache.solr
+solr-solrj
+${solr.version}
+provided
--- End diff --
Shouldn't this be compile scope here so that SolrJ gets included in the
client API NAR?
It could then probably be marked as provided in the processors pom since
the processors would get it through the parent NAR.
---
[GitHub] nifi pull request #3041: NIFI-5224 Added SolrClientService.
GitHub user MikeThomsen opened a pull request: https://github.com/apache/nifi/pull/3041 NIFI-5224 Added SolrClientService. 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- where 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-5224 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3041.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 #3041 commit ca5c881b1d2ceee82bb7d6700191eb209612adbc Author: Mike Thomsen Date: 2018-10-01T13:53:16Z NIFI-5224 Added SolrClientService. ---
