You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/10/10 22:19:00 UTC
[GitHub] [solr] joshgog opened a new pull request, #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
joshgog opened a new pull request, #1061:
URL: https://github.com/apache/solr/pull/1061
https://issues.apache.org/jira/browse/SOLR-15748
<!--
_(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 Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
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
Created a v2 equivalent of 'CLUSTERSTATUS' command under the v1 /solr/admin/collections endpoint.
# Solution
Created an endpoint path = {"/c?", "/collections?"}
# Tests
Tested mapping to the API endpoint.
# Checklist
Please review the following and check all that apply:
- [x] 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.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] 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)
- [x] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija merged pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija merged PR #1061:
URL: https://github.com/apache/solr/pull/1061
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r994341413
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +257,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
Since I'm still learning the code base and how Solr works, I'll have to look into how the `CloudSolrClient `works to better understand your point and put across my thoughts.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1278090152
CLUSTERSTATUS returns a superset of the information returned by list. That is, LIST returns the collection names only. CLUSTERSTATUS returns not just the collection names, but the shards and replicas that make them up, each of their "statuses", the nodes that make up the cluster, etc.
Are you seeing them return the same info? If so, can you share `curl` commands or something similar to test out?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r993652895
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +263,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster/cluster-status", permission = COLL_READ_PERM)
Review Comment:
> Did you pick this because "/cluster" was already taken by the `getCluster` method down below? Or was there another reason?
Yes that's the main reason, "/cluster" had already been mapped to a method.
> For reasons that I don't quite understand, the v2 API has two endpoints that expose the "List Collections" functionality: `ClusterAPI.getCluster` which we see below uses the "/cluster" path, and `CollectionsAPI.getCollections` [here](https://github.com/apache/solr/blob/c99af207c761ec34812ef1cc3054eb2804b7448b/solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java#L79) which uses the path "/collections".
Exactly. It was a challenge understing the same while working on the issue.
> There's absolutely no reason for that afaict, and "cluster status" is a very natural thing to expose at "/cluster". So IMO we should be safe to delete `getCluster` in this PR.
Will do that
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1278018498
I did also check, the ref guide does mention `/api/collections` for listing out collections!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1277941680
(Tests and 'check' pass for me 👍 )
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1278029259
It seems the action `LIST` and action `CLUSTERSTATUS` return the same result. No?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r993620386
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +263,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster/cluster-status", permission = COLL_READ_PERM)
Review Comment:
[+1] The method body itself here looks great 👍
[Q] My only question is about the path string you chose: "/cluster/cluster-status".
Did you pick this because "/cluster" was already taken by the `getCluster` method down below? Or was there another reason?
Assuming the main factor was the conflict with `getCluster`, I'd vote to nuke that method and change the path string here to be "/cluster".
For reasons that I don't quite understand, the v2 API has two endpoints that expose the "List Collections" functionality: `ClusterAPI.getCluster` which we see below uses the "/cluster" path, and `CollectionsAPI.getCollections` [here](https://github.com/apache/solr/blob/c99af207c761ec34812ef1cc3054eb2804b7448b/solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java#L79) which uses the path "/collections".
There's absolutely no reason for that afaict, and "cluster status" is a very natural thing to expose at "/cluster". So IMO we should be safe to delete `getCluster` in this 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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1276932256
> 1. I noticed you left the `./gradlew check` box unchecked on the checklist in the PR description. Running `check` isn't strictly required, as most committers will run it themselves before merging code. But it does help give your reviewer a bit of extra confidence as it signals: "Hey, the code quality checks and tests and all that passed for me, so this should be ready to review". So if you're willing, feel free to give that a try when you get a few minutes and let me know how it went.
There was one failed test. JWT security issues. I'm rerunning the check to note the details of the error. Where is the test log located? To avoid rerunning the check.
> 2\. End-user-visible changes in Solr usually get mentioned in a "change-log" file that lives at `solr/CHANGES.txt`. This file exists both to give users an idea of what might've changed release-to-release, but also as a way to give credit and recognition to all the contributors like you who helped. If you get a few minutes, could you document this change by adding a bullet point under the "Improvements" section for the 9.2.0 release? Alternately, if you don't have time or aren't sure what to write, I'm happy to draft that myself if you can tell me what name you'd like to be credited under. (Some people use their full name, some people use a pseudonym or username, etc.)
I have documented the changes.
> 3\. Lastly, since we're changing a Solr API we should probably update the documentation anywhere it might be mentioned. Solr's main documentation, the "Solr Reference Guide" lives alongside the code in this repo at `solr/solr-ref-guide`. It can be built using the command `./gradlew buildLocalSite` - more detail on this in the README [here](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/README.adoc). The one place in particular that I know needs updated is the section [here](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc#clusterstatus-cluster-status). Could you take a crack at updating that and let me know how it goes?
I have updated the reference guide
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1276957118
> There was one failed test. JWT security issues. I'm rerunning the check to note the details of the error. Where is the test log located? To avoid rerunning the check.
The ./gradlew check rerun was successful. No failed tests
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r994302604
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +257,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
The `COLL_READ_PERM` (`collection-admin-read`) permission here stems from when CLUSTERSTATUS was a collection-api command.
The content of clusterstatus is mostly collection data, i.e. a list of collections, their shards and replicas and their status. So I'm OK with keeping this permission although it is now located under`/api/cluster`. I'm thinking of the `CloudSolrClient` SolrJ client that will read cluster status in order to route index/search requests.
Any thoughts?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1273896187
:warning: **314 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/solr/01GF1ZNPBNHTNAP2YW9FSC3SS8?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20solr) for more details.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r994844733
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +257,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
Are you getting at the fact that COLL_READ_PERM/collection-admin-read is a bad name in the v2 world where /admin/collections no longer exists? Or are you suggesting that it might be nice to give "cluster status" a different permission group so that CSC's can be setup to require fewer permissions? Or something else altogether?
If the former, then I totally agree. I think it still makes sense to have a permission group _like_ COLL_READ_PERM going forward, but the name would need to be rethought. Regardless of the underlying api paths or the permission name though, I think users will still find it valuable to group various admin-read functionality in a single permission-group.
In terms of having a "slimmer" predefined permission group for HttpClusterStateProvider-based CloudSolrClients to use, that seems like it might be useful. Though, to play devil's advocate, IMO HttpClusterStateProvider is sort of a minority use-case, and users that want to run a HCSP without giving it full COLL_READ_PERMs can already do so by defining "custom permissions". So I could go either way I guess 🤷
In either case - IMO making changes to our predefined permissions is definitely a bigger task that we'd want to be a separate JIRA for sure.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r994357333
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +257,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
I'm not expecting you to have an opinion on this detail. Solr's security framework is a bit involved :) But if a committer has an opinion I'm happy to hear it.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] joshgog commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1278366352
> Are you seeing them return the same info?
II had not tested LIST. They do return different info
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1061:
URL: https://github.com/apache/solr/pull/1061#discussion_r994844733
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -261,6 +257,13 @@ public void getNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
rsp.add("nodes", getCoreContainer().getZkController().getClusterState().getLiveNodes());
}
+ @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
Are you getting at the fact that COLL_READ_PERM/collection-admin-read is a bad name in the v2 world where /admin/collections no longer exists? Or are you suggesting that it might be nice to give "cluster status" a different permission group so that CSC's can be setup to require fewer permissions? Or something else altogether?
If the former, then I totally agree. I think it still makes sense to have a permission group _like_ COLL_READ_PERM going forward, but the name would need to be rethought. Regardless of the underlying api paths or the permission name though, I think users will still find it valuable to group various admin-read functionality in a single permission-group.
In terms of having a "slimmer" predefined permission group for HttpClusterStateProvider-based CloudSolrClients to use, that seems like it might be useful. Though, to play devil's advocate, IMO HttpClusterStateProvider is sort of a minority use-case, and users that want to run a HCSP without giving it full COLL_READ_PERMs can already do so by defining "custom permissions". So I could go either way I guess 🤷
In either case - making changes to our predefined permissions is definitely a bigger task that we'd want to be a separate JIRA for sure.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1061: SOLR-15748 : Created v2 equivalent of v1 'CLUSTERSTATUS' command
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1061:
URL: https://github.com/apache/solr/pull/1061#issuecomment-1276409958
Hey @joshgog - thanks for the PR; looks good so far! A few quick questions for you:
1. I noticed you left the `./gradlew check` box unchecked on the checklist in the PR description. Running `check` isn't strictly required, as most committers will run it themselves before merging code. But it does help give your reviewer a bit of extra confidence as it signals: "Hey, the code quality checks and tests and all that passed for me, so this should be ready to review". So if you're willing, feel free to give that a try when you get a few minutes and let me know how it went.
2. End-user-visible changes in Solr usually get mentioned in a "change-log" file that lives at `solr/CHANGES.txt`. This file exists both to give users an idea of what might've changed release-to-release, but also as a way to give credit and recognition to all the contributors like you who helped. If you get a few minutes, could you document this change by adding a bullet point under the "Improvements" section for the 9.2.0 release? Alternately, if you don't have time or aren't sure what to write, I'm happy to draft that myself if you can tell me what name you'd like to be credited under. (Some people use their full name, some people use a pseudonym or username, etc.)
3. Lastly, since we're changing a Solr API we should probably update the documentation anywhere it might be mentioned. Solr's main documentation, the "Solr Reference Guide" lives alongside the code in this repo at `solr/solr-ref-guide`. It can be built using the command `./gradlew buildLocalSite` - more detail on this in the README [here](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/README.adoc). The one place in particular that I know needs updated is the section [here](https://github.com/apache/solr/blob/main/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc#clusterstatus-cluster-status). Could you take a crack at updating that and let me know how it goes?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org