[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org