gerlowskija commented on code in PR #2229:
URL: https://github.com/apache/solr/pull/2229#discussion_r1470500298
##########
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java:
##########
@@ -237,8 +237,10 @@ public Map<String, SolrPackageInstance>
getPackagesDeployed(String collection) {
NamedList<Object> result =
solrClient.request(
new GenericSolrRequest(
- SolrRequest.METHOD.GET,
- PackageUtils.getCollectionParamsPath(collection) +
"/PKG_VERSIONS"));
+ SolrRequest.METHOD.GET,
+ PackageUtils.getCollectionParamsPath(collection) +
"/PKG_VERSIONS")
+ .setRequiresCollection(
+ false) /* Making a collection request, but already baked
into path */);
Review Comment:
We could - basically that'd involve changing
`PackageUtils.getCollectionParamsPath` to return a value of `/config/params`
instead of the current value
`/api/collections/someCollectionName/config/params`. The method has 8 or 10
call sites, so we'd have to restructure the logic in each of those places.
It's not that big of a lift; I'm happy to go that route if it'd be your
preference? Lmk.
That said, I have a slight preference for skipping this now. Mostly
because, as you mentioned - this code really shouldn't be using GSR in any
case. No point spending time reworking the path-building logic now, if down
the line we're gonna replace all this code anyways with a more appropriate
SolrRequest impl.
##########
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java:
##########
@@ -237,8 +237,10 @@ public Map<String, SolrPackageInstance>
getPackagesDeployed(String collection) {
NamedList<Object> result =
solrClient.request(
new GenericSolrRequest(
- SolrRequest.METHOD.GET,
- PackageUtils.getCollectionParamsPath(collection) +
"/PKG_VERSIONS"));
+ SolrRequest.METHOD.GET,
+ PackageUtils.getCollectionParamsPath(collection) +
"/PKG_VERSIONS")
+ .setRequiresCollection(
+ false) /* Making a collection request, but already baked
into path */);
Review Comment:
We could - basically that'd involve changing
`PackageUtils.getCollectionParamsPath` to return a value of `/config/params`
instead of the current value
`/api/collections/someCollectionName/config/params`. The method has 8 or 10
call sites, so we'd have to restructure the logic in each of those places.
It's not that big of a lift; I'm happy to go that route if it'd be your
preference? Lmk.
That said, I have a slight preference for skipping this now. Mostly
because, as you mentioned - this code really shouldn't be using GSR at all. No
point spending time reworking the path-building logic now, if down the line
we're gonna replace all this code anyways with a more appropriate SolrRequest
impl.
--
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]
