dsmiley commented on code in PR #2689:
URL: https://github.com/apache/solr/pull/2689#discussion_r1742021681


##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java:
##########
@@ -169,6 +169,10 @@ public synchronized SolrClient getHttpSolrClient(String 
baseUrl) {
     return client;
   }
 

Review Comment:
   needs javadoc like: "The default client to use to for internal Solr 
communication.  There are others for certain categories of work.".



##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -347,22 +348,27 @@ PublicKey fetchPublicKeyFromRemote(String nodename) {
     HttpEntity entity = null;
     try {
       String uri = url + PublicKeyHandler.PATH + "?wt=json&omitHeader=true";
+      ModifiableSolrParams solrParams = new ModifiableSolrParams();

Review Comment:
   glad to see you convert to use a SolrClient :-).  It'll be nice to have the 
observability benefits.



##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -652,6 +652,10 @@ public Lookup<AuthSchemeProvider> getAuthSchemeRegistry() {
       
pkiAuthenticationSecurityBuilder.getHttpClientBuilder(HttpClientUtil.getHttpClientBuilder());
       shardHandlerFactory.setSecurityBuilder(pkiAuthenticationSecurityBuilder);
       updateShardHandler.setSecurityBuilder(pkiAuthenticationSecurityBuilder);
+      if (solrClientCache instanceof ServerSolrClientCache) {

Review Comment:
   Declare it of this type so we don't cast.



##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -347,22 +348,27 @@ PublicKey fetchPublicKeyFromRemote(String nodename) {
     HttpEntity entity = null;
     try {
       String uri = url + PublicKeyHandler.PATH + "?wt=json&omitHeader=true";
+      ModifiableSolrParams solrParams = new ModifiableSolrParams();
+      solrParams.add("wt", "json");
+      solrParams.add("omitHeader", "true");
+
+      GenericSolrRequest request =
+          new GenericSolrRequest(SolrRequest.METHOD.GET, 
PublicKeyHandler.PATH, solrParams);
+
       log.debug("Fetching fresh public key from: {}", uri);
-      HttpResponse rsp =
-          cores
-              .getUpdateShardHandler()
-              .getDefaultHttpClient()
-              .execute(new HttpGet(uri), 
HttpClientUtil.createNewHttpClientRequestContext());
-      entity = rsp.getEntity();
-      byte[] bytes = EntityUtils.toByteArray(entity);
-      Map<?, ?> m = (Map<?, ?>) Utils.fromJSON(bytes);
-      String key = (String) m.get("key");
-      if (key == null) {
-        log.error("No key available from {}{}", url, PublicKeyHandler.PATH);
-        return null;
-      } else {
-        log.info("New key obtained from node={}, key={}", nodename, key);
+
+      String key;
+      try (Http2SolrClient solrClient = new 
Http2SolrClient.Builder(url).build()) {

Review Comment:
   why create a new client when you can use the default client?



-- 
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...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to