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 2020/03/30 15:55:52 UTC

[GitHub] [nifi] mkazia opened a new pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

mkazia opened a new pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170
 
 
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
   #### Description of PR
   
   _ Fixes bug NIFI-7294._
   Address deprecation issues in solrj and httpclient
   
   Some calls to deprecated methods in httpclient were resulting in
   UnsupportedOperationException. Use the new API calls in both httpclient
   and solrj. Add an integration test to include test coverage for
   org.apache.nifi.processors.solr.SolrUtils.createClient
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [x] 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? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [x] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [x] Have you written or updated unit tests to verify your changes?
   - [x] Have you verified that the full build is successful on both JDK 8 and JDK 11?
   - [ ] 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.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mkazia commented on issue #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
mkazia commented on issue #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#issuecomment-606132267
 
 
   Closing this so that I can open a new clean PR with just one commit.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400330050
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/TestGetSolr.java
 ##########
 @@ -48,8 +48,8 @@
 import java.util.Locale;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   assertThat in junit is deprecated. See https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertThat(T,%20org.hamcrest.Matcher)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on issue #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
bbende commented on issue #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#issuecomment-606110930
 
 
   A couple of things from reviewing...
   
   1) It seems like this branch includes the commits for the previous work on upgrading SolrJ, there should only be one commit in this PR for the new changes for NIFI-7294.
   
   2) I think there is a potential issue with our use of the `HttpClientUtil` static methods... since we set static values in there, once one Solr processor has been created that is configured with Kerberos or SSL, then that static state has been set in `HttpClientUtil `and now if someone made a second Solr processor that wasn't using Kerberos or SSL, that stuff would still be set in `HttpClientUtil`. It looks like there is a method `resetHttpClientBuilder()` that we should call at the beginning of `createSolrClient`, but we also need to ensure we are inside a synchronized block so that two different Solr processors don't alter `HttpClientUtil` at the same time. We can do that by making `createSolrClient` synchronized.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400326224
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/TestGetSolr.java
 ##########
 @@ -48,8 +48,8 @@
 import java.util.Locale;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   Is this change required?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mkazia closed pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
mkazia closed pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400330184
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/QuerySolrIT.java
 ##########
 @@ -36,25 +39,29 @@
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.xmlunit.matchers.CompareMatcher;
 
+import javax.net.ssl.SSLContext;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.SimpleDateFormat;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   assertThat in junit is deprecated. See https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertThat(T,%20org.hamcrest.Matcher)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400326691
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/QuerySolrIT.java
 ##########
 @@ -36,25 +39,29 @@
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.xmlunit.matchers.CompareMatcher;
 
+import javax.net.ssl.SSLContext;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.SimpleDateFormat;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   Is this change of class required?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
mkazia commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400329921
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/TestQuerySolr.java
 ##########
 @@ -48,9 +48,9 @@
 import java.util.Map;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   assertThat in junit is deprecated. See https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertThat(T,%20org.hamcrest.Matcher)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient

Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4170: NIFI-7294 Address deprecation issues in solrj and httpclient
URL: https://github.com/apache/nifi/pull/4170#discussion_r400326052
 
 

 ##########
 File path: nifi-nar-bundles/nifi-solr-bundle/nifi-solr-processors/src/test/java/org/apache/nifi/processors/solr/TestQuerySolr.java
 ##########
 @@ -48,9 +48,9 @@
 import java.util.Map;
 import java.util.TimeZone;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 
 Review comment:
   I believe this change is not required, right?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services