epugh commented on code in PR #4773:
URL: https://github.com/apache/solr/pull/4773#discussion_r3817383068
##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -1993,6 +1993,11 @@ public static void verbose(Object... args) {
/**
* Get the request handler registered to a given name.
*
+ * <p>A {@code null} handlerName resolves to the core's default request
handler (whichever handler
+ * is aliased to the empty string, normally the one registered at "/select",
or "standard" for
+ * legacy configs) rather than returning {@code null}; see {@link
+ * RequestHandlers#initHandlersFromConfig}.
Review Comment:
Fun!
##########
solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java:
##########
@@ -351,7 +352,7 @@ void doAddDoc(String id) throws Exception {
}
void doRTG(String ids) throws Exception {
- doQuery(ids, "qt", "/get", "ids", ids);
+ doQuery(StrUtils.splitSmart(ids, ",", true), "/get", "ids", ids);
Review Comment:
Double ids? On phone so maybe not getting full context...
##########
solr/core/src/test/org/apache/solr/servlet/CacheHeaderTest.java:
##########
@@ -58,7 +58,7 @@ public static void beforeTest() throws Exception {
@Test
public void testCacheVetoException() throws Exception {
- String url = getSelectUrl("q", "xyz_ignore_exception:solr", "qt",
"standard");
Review Comment:
so much nicer!
##########
solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java:
##########
@@ -626,8 +625,7 @@ public void testJustJohnJavabin() throws Exception {
SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(johnTwoFL, rsp));
- SolrQueryResponse response =
- h.queryAndResponse(johnTwoFL.getParams().get(CommonParams.QT),
johnTwoFL);
+ SolrQueryResponse response = h.queryAndResponse(null, johnTwoFL);
Review Comment:
I never like seeing a `null` as a parameter, but maybe me. We have a couple
of calls like this, would it be worth a `h.queryAndResponse(johnTwoFL)`
signature?
##########
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java:
##########
@@ -2179,6 +2180,18 @@ void doQuery(String expectedDocs, String... queryParams)
throws Exception {
assertEquals(expectedIds, obtainedIds);
}
+ void doQuery(List<String> expectedIds, String path, String... queryParams)
throws Exception {
Review Comment:
ah, here is the source of the triple "id" signature. I thought `doQuery`
was for making queries, not a validation method! `doQueryAndAssertResults`?
##########
solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java:
##########
@@ -351,7 +352,7 @@ void doAddDoc(String id) throws Exception {
}
void doRTG(String ids) throws Exception {
- doQuery(ids, "qt", "/get", "ids", ids);
+ doQuery(StrUtils.splitSmart(ids, ",", true), "/get", "ids", ids);
Review Comment:
looking again, i don't quite get this `doQuery` methods ;-). I see ids
three times!
--
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]
