You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/21 19:00:20 UTC
[GitHub] [lucene-solr] NazerkeBS opened a new pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
NazerkeBS opened a new pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
Please provide a short description of the changes you're making with this pull request.
# Solution
Please provide a short description of the approach taken to implement your solution.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `master` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r567809670
##########
File path: solr/webapp/web/js/angular/services.js
##########
@@ -58,10 +58,10 @@ solrAdminServices.factory('System',
}])
.factory('Logging',
['$resource', function($resource) {
- return $resource('admin/info/logging', {'wt':'json', 'nodes': 'all', '_':Date.now()}, {
+ return $resource('admin/info/logging', {'wt':'json', '_':Date.now()}, {
"events": {params: {since:'0'}},
"levels": {},
- "setLevel": {}
+ "setLevel": {'nodes': 'all'}
Review comment:
The JavaScript framework we use here is completely unknown to me. Does this change make sense to you @janhoy ?
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley merged pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230
----------------------------------------------------------------
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
[GitHub] [lucene-solr] NazerkeBS commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
NazerkeBS commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r568061888
##########
File path: solr/webapp/web/js/angular/services.js
##########
@@ -58,10 +58,10 @@ solrAdminServices.factory('System',
}])
.factory('Logging',
['$resource', function($resource) {
- return $resource('admin/info/logging', {'wt':'json', 'nodes': 'all', '_':Date.now()}, {
+ return $resource('admin/info/logging', {'wt':'json', '_':Date.now()}, {
"events": {params: {since:'0'}},
"levels": {},
- "setLevel": {}
+ "setLevel": {'nodes': 'all'}
Review comment:
I debugged it on Admin UI and it works without `params`. But I updated my PR to use `params` as other handlers also use `params` in this file .
----------------------------------------------------------------
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
[GitHub] [lucene-solr] NazerkeBS commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
NazerkeBS commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r563636966
##########
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:
SystemInfoHandler is doing similar to this logic;
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r565448485
##########
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:
@NazerkeBS ?
----------------------------------------------------------------
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
[GitHub] [lucene-solr] janhoy commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r567431423
##########
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 think nodes=all should be added only to the `setLevel` call. Should be easy to check in browser debugger that it sends correct request.
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r565608099
##########
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:
Okay; I see you improved that from an earlier commit.
@janhoy maybe you could take a look at this please. Should nodes=all should only be added when setLevel is submitted, not for all logging handler interactions. Do you think this is done correctly here?
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r564755969
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java
##########
@@ -41,7 +46,9 @@
public class AdminHandlersProxyTest extends SolrCloudTestCase {
private CloseableHttpClient httpClient;
private CloudSolrClient solrClient;
-
+ private GenericSolrRequest req;
Review comment:
Why define these here? Always define variables to the most limited scope possible. SolrClient makes sense here because it has the lifecycle of the test case (the class).
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java
##########
@@ -91,6 +98,32 @@ public void proxyMetricsHandlerAllNodes() throws IOException, SolrServerExceptio
assertNotNull(((NamedList)nl.get(nl.getName(1))).get("metrics"));
}
+ @Test
+ public void proxyLoggingHandlerAllNodes() throws IOException, SolrServerException {
+ CollectionAdminRequest.createCollection("collection", "conf", 2, 2).process(solrClient);
+
+ mparams = new ModifiableSolrParams();
+ mparams.set(CommonParams.QT, "/admin/logging");
+ mparams.set("nodes", "all");
+ mparams.set("set", "com.codahale.metrics.jmx.JmxReporter:WARN");
+ req = new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/logging", mparams);
+ req.process(solrClient, "collection");
+
+ Set<String> nodes = solrClient.getClusterStateProvider().getLiveNodes();
+ nodes.forEach(node -> {
+ mparams.clear();
+ mparams.set("nodes", node);
+ req = new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/logging", mparams);
+ try {
+ rsp = req.process(solrClient, "collection");
+ } catch (Exception e) {
+ fail("Exception while proxying request to node " + node);
Review comment:
If above you used a standard for loop, you wouldn't be forced to catch this exception, and it'd be more succinct. Personally, I only use Collection.forEach lambda for a one-liner or something short and no exception propagation concern.
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java
##########
@@ -91,6 +98,32 @@ public void proxyMetricsHandlerAllNodes() throws IOException, SolrServerExceptio
assertNotNull(((NamedList)nl.get(nl.getName(1))).get("metrics"));
}
+ @Test
+ public void proxyLoggingHandlerAllNodes() throws IOException, SolrServerException {
+ CollectionAdminRequest.createCollection("collection", "conf", 2, 2).process(solrClient);
+
+ mparams = new ModifiableSolrParams();
Review comment:
BTW, Solr test infrastructure (SolrTestCaseJ4) defines a params(...) method that is more succinct. This is fine though!
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java
##########
@@ -49,25 +49,22 @@
@SuppressWarnings({"rawtypes"})
private LogWatcher watcher;
- private CoreContainer cc;
+ private final CoreContainer cc;
public LoggingHandler(CoreContainer cc) {
this.cc = cc;
this.watcher = cc.getLogging();
}
public LoggingHandler() {
Review comment:
Why would this constructor be used? If it's just a test, then couldn't it use the one that takes a cc but pass null? I just did a search and see that this constructor would be used when the handler is loaded on a core -- ImplicitPlugins.json. Ok... I suppose I should file an issue to remove these handlers from ImplicitPlugins where they don't make sense.
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r565608099
##########
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:
Okay; I see you improved that from an earlier commit.
@janhoy maybe you could take a look at this please. `nodes=all` is intended only for when setLevel is submitted, not for all logging handler interactions. Do you think this is done correctly here?
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r568204361
##########
File path: solr/CHANGES.txt
##########
@@ -69,6 +69,8 @@ Improvements
* SOLR-14949: Docker: Ability to customize the FROM image when building.
(Houston Putman)
+* SOLR-15011: /admin/logging handler should be able to configure logs on all nodes (Nazerke Seidan, David Smiley)
Review comment:
```suggestion
* SOLR-15011: /admin/logging handler will now propagate setLevel (log threshold) to all nodes
when told to. The admin UI now tells it to. (Nazerke Seidan, David Smiley)
```
----------------------------------------------------------------
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
[GitHub] [lucene-solr] NazerkeBS commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
NazerkeBS commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r563636966
##########
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:
SystemInfoHandler is doing similar to this logic;
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by 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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r563971668
##########
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:
SIH is doing what I suggest, which is different than what the PR is doing.
----------------------------------------------------------------
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
[GitHub] [lucene-solr] NazerkeBS commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
NazerkeBS commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r565562275
##########
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:
@dsmiley, I set nodes=all to only one place which is in services.js file
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r567447802
##########
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 verified this is a problem now! Thanks for the tip Jan; I forgot how easy it is to verify :-)
----------------------------------------------------------------
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
[GitHub] [lucene-solr] NazerkeBS commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
NazerkeBS commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r567687823
##########
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:
Thank Jan and David! Just updated the PR.
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r563971668
##########
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:
SIH is doing what I suggest, which is different than what the PR is doing.
----------------------------------------------------------------
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
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r568204361
##########
File path: solr/CHANGES.txt
##########
@@ -69,6 +69,8 @@ Improvements
* SOLR-14949: Docker: Ability to customize the FROM image when building.
(Houston Putman)
+* SOLR-15011: /admin/logging handler should be able to configure logs on all nodes (Nazerke Seidan, David Smiley)
Review comment:
```suggestion
* SOLR-15011: /admin/logging handler will now propagate setLevel (log threshold) to all nodes
when told to. The admin UI now tells it to. (Nazerke Seidan, David Smiley)
```
----------------------------------------------------------------
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
[GitHub] [lucene-solr] janhoy commented on a change in pull request #2230: SOLR-15011: /admin/logging handler is configured logs to all nodes
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #2230:
URL: https://github.com/apache/lucene-solr/pull/2230#discussion_r568007698
##########
File path: solr/webapp/web/js/angular/services.js
##########
@@ -58,10 +58,10 @@ solrAdminServices.factory('System',
}])
.factory('Logging',
['$resource', function($resource) {
- return $resource('admin/info/logging', {'wt':'json', 'nodes': 'all', '_':Date.now()}, {
+ return $resource('admin/info/logging', {'wt':'json', '_':Date.now()}, {
"events": {params: {since:'0'}},
"levels": {},
- "setLevel": {}
+ "setLevel": {'nodes': 'all'}
Review comment:
It’s Angular 1. Looks right but I’m not sure if there should be a `params: ` there as well. Testing will tell..
----------------------------------------------------------------
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