bszabo97 commented on code in PR #1182:
URL: https://github.com/apache/solr/pull/1182#discussion_r1159424945
##########
solr/core/src/java/org/apache/solr/packagemanager/DefaultPackageRepository.java:
##########
@@ -101,16 +106,21 @@ public Path download(String artifactName) throws
SolrException, IOException {
}
private void initPackages() {
- try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
+ try (Http2SolrClient client =
+ new Http2SolrClient.Builder(repositoryURL).useHttp1_1(true).build()) {
Review Comment:
@epugh as I was searching for this in the codebase, the only place where I
could find usage for the jetty http client was the Http2SolrClient class. I may
have missed something, but I believe we make use of the apache client wherever
we just need to do a simple get. If you know of any place where we do it with
the jetty client please help me out and let me know!
@dsmiley good idea for the comment, I will add it there!
##########
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java:
##########
@@ -727,22 +726,19 @@ private Map<String, String>
getCollectionParameterOverrides(
@SuppressWarnings({"rawtypes", "unchecked"})
Map<String, String> getPackageParams(String packageName, String collection) {
try {
+ NamedList<Object> response =
+ solrClient.request(
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ PackageUtils.getCollectionParamsPath(collection) +
"/packages",
+ new ModifiableSolrParams()));
return (Map<String, String>)
- ((Map)
- ((Map)
- ((Map)
- PackageUtils.getJson(
- solrClient.getHttpClient(),
- solrBaseUrl
- +
PackageUtils.getCollectionParamsPath(collection)
- + "/packages",
- Map.class)
- .get("response"))
- .get("params"))
- .get("packages"))
- .get(packageName);
+ response._get("/response/params/packages/" + packageName,
Collections.emptyMap());
} catch (Exception ex) {
// This should be because there are no parameters. Be tolerant here.
+ if (log.isWarnEnabled()) {
Review Comment:
I was under the impression that I put it there because the validation did
not pass, but I will remove it. Good to know this information!
##########
solr/core/src/java/org/apache/solr/util/PackageTool.java:
##########
@@ -360,15 +365,18 @@ private String getZkHost(CommandLine cli) throws
Exception {
String zkHost = cli.getOptionValue("zkHost");
if (zkHost != null) return zkHost;
- String systemInfoUrl = solrUrl + "/admin/info/system";
- CloseableHttpClient httpClient = SolrCLI.getHttpClient();
- try {
+ try (SolrClient solrClient = getSolrClient(solrUrl)) {
// hit Solr to get system info
- Map<String, Object> systemInfo = SolrCLI.getJson(httpClient,
systemInfoUrl, 2, true);
+ NamedList<Object> systemInfo =
+ solrClient.request(
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ CommonParams.SYSTEM_INFO_PATH,
+ new ModifiableSolrParams()));
Review Comment:
Great idea, done. Also changed this in all places where we used the
constructor with null or empty ModifiableSolrParams
##########
solr/core/src/java/org/apache/solr/util/SolrCLI.java:
##########
@@ -1316,19 +1076,20 @@ protected void runCloudTool(CloudLegacySolrClient
cloudSolrClient, CommandLine c
q = new SolrQuery("*:*");
q.setRows(0);
q.set(DISTRIB, "false");
- try (HttpSolrClient solr = new
HttpSolrClient.Builder(coreUrl).build()) {
-
- String solrUrl = solr.getBaseURL();
-
- qr = solr.query(q);
+ int lastSlash = coreUrl.substring(0, coreUrl.length() -
1).lastIndexOf('/');
+ try (var solrClient = getSolrClient(coreUrl.substring(0,
lastSlash))) {
+ qr = solrClient.query(coreUrl.substring(lastSlash + 1,
coreUrl.length() - 1), q);
Review Comment:
Great suggestion, it looks much better like this, thank you!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
