dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r562635350
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java
##########
@@ -63,6 +65,9 @@ public void inform(SolrCore core) {
if (watcher == null) {
watcher = core.getCoreContainer().getLogging();
}
+ if (cc == null) {
Review comment:
does this actually happen (how?). Otherwise, let's not and furthermore
declare cc as final.
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java
##########
@@ -151,6 +156,9 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
rsp.add("loggers", info);
}
rsp.setHttpCaching(false);
+ if (cc != null && AdminHandlersProxy.maybeProxyToNodes(req, rsp, cc)) {
Review comment:
Why do this at the end of this method instead of at the top (along with
disabling http caching)? By doing it at the end, if nodes=FOO yet the current
node is BAR, it would do work it shouldn't be doing; right? Notice where the
other callers of AdminHandlersProxy.maybeProxyToNodes put their logic.
##########
File path: solr/webapp/web/js/angular/services.js
##########
@@ -58,7 +58,7 @@ solrAdminServices.factory('System',
}])
.factory('Logging',
['$resource', function($resource) {
- return $resource('admin/info/logging', {'wt':'json', '_':Date.now()}, {
+ return $resource('admin/info/logging', {'wt':'json', 'nodes': 'all',
'_':Date.now()}, {
Review comment:
I'm very unfamiliar with the admin UI code structure. Can you explain
why nodes=all needed to be set in two places?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
