dsmiley commented on code in PR #2645:
URL: https://github.com/apache/solr/pull/2645#discussion_r1717131530
##########
solr/core/src/java/org/apache/solr/handler/admin/ColStatus.java:
##########
@@ -176,48 +182,50 @@ public void getColStatus(NamedList<Object> results) {
if (url == null) {
continue;
}
- try (SolrClient client = solrClientCache.getHttpSolrClient(url)) {
- ModifiableSolrParams params = new ModifiableSolrParams();
- params.add(CommonParams.QT, "/admin/segments");
- params.add(FIELD_INFO_PROP, "true");
- params.add(CORE_INFO_PROP, String.valueOf(withCoreInfo));
- params.add(SIZE_INFO_PROP, String.valueOf(withSizeInfo));
- params.add(RAW_SIZE_PROP, String.valueOf(withRawSizeInfo));
- params.add(RAW_SIZE_SUMMARY_PROP,
String.valueOf(withRawSizeSummary));
- params.add(RAW_SIZE_DETAILS_PROP,
String.valueOf(withRawSizeDetails));
- if (samplingPercent != null) {
- params.add(RAW_SIZE_SAMPLING_PERCENT_PROP,
String.valueOf(samplingPercent));
- }
- QueryRequest req = new QueryRequest(params);
- NamedList<Object> rsp = client.request(req);
- rsp.remove("responseHeader");
- leaderMap.add("segInfos", rsp);
- NamedList<?> segs = (NamedList<?>) rsp.get("segments");
- if (segs != null) {
- for (Map.Entry<String, ?> entry : segs) {
- NamedList<Object> fields =
- (NamedList<Object>) ((NamedList<Object>)
entry.getValue()).get("fields");
- if (fields != null) {
- for (Map.Entry<String, Object> fEntry : fields) {
- Object nc = ((NamedList<Object>)
fEntry.getValue()).get("nonCompliant");
- if (nc != null) {
- nonCompliant.add(fEntry.getKey());
+ if (getSegments) {
+ try (SolrClient client = solrClientCache.getHttpSolrClient(url)) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.add(CommonParams.QT, "/admin/segments");
+ params.add(FIELD_INFO_PROP, "true");
Review Comment:
curious to see "true" here instead of withFieldInfo. I know this isn't a
change you brought about but it's in scope to either fix or comment justifying.
##########
solr/core/src/java/org/apache/solr/handler/admin/ColStatus.java:
##########
@@ -93,8 +93,14 @@ public void getColStatus(NamedList<Object> results) {
if (withRawSizeSummary || withRawSizeDetails) {
withRawSizeInfo = true;
}
- if (withFieldInfo || withSizeInfo) {
- withSegments = true;
+ boolean getSegments = false;
Review Comment:
FWIW I think `getSegments` is named fine, but so are your suggestions. I
don't think I've ever confused a method for a variable/field because the syntax
involved would be nonsensical if reversed. The line commented on here would
unfathomably refer to a method (`type varName = value`). Lines of code
referencing the var won't have calling parenthesis so wouldn't be a method
call. And would be used in boolean ways (`if (getSegments)`) not a more
interesting structure.
--
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]
