SOLR-17359: Move Zk Arg parsing into Java Code #2593
Conversation
github-actions
bot
added
documentation
|
Okay, I changed the short cut "-s" to mean Solr URL instead of "shards". however, in 9x "-s" means shards.. I am thinking that maybe I change allt he tests to use "--shards", and when I backport to 9x, I don't make "-s" mean solr url, but leave it as shards parameter? |
|
FYI, for folks tracking this, @rahulgoswami has started testing this PR on Windows! Thanks @rahulgoswami ... He has validated |
|
Update so far:
Yet to test: zk rm, zk mv, zk mkroot, and some more of zk cp . That should cover everything talked about in the documentation (https://solr.apache.org/guide/solr/latest/deployment-guide/solr-control-script-reference.html). There are more if I dig into SolrCLI.java. Do those need testing as well?
Exception in thread "main" java.lang.StackOverflowError
ERROR: Error downloading files from zookeeper path /configs/null to C:\Work\Git\command-testing\conf
====
ERROR: Index 1 out of bounds for length 1 |
|
I'm going to try out some of the examples on my Mac, just to see which are OS specific and which are just part of the code movement not being right. I'll also take a stab at adding more bats style tests to duplicate what you've added. I think the more testing on Solr on Windows, the better! There has been a LOT of churn. |
… clear. Eg: >solr zk ls
|
Okay, just fixed the downconfig and upconfig commands! |
|
@rahulgoswami would you mind giving another pass through! I fixed a BUNCH of bugs that you found, thank you for the testing. Plus, I updated this branch with other changes that have happened in main that I think fix the RunExampleTool bugs you saw.... |
|
Pulled in the latest changes on the branch SOLR-17359 and rebuilt. This covers testing of all commands listed in documentation (https://solr.apache.org/guide/solr/latest/deployment-guide/solr-control-script-reference.html#zookeeper-operations). Will try to play with a few more not listed in the documentation, but that show up under help.
ERROR: Index 0 out of bounds for length 0
ERROR: Index 0 out of bounds for length 0 >solr zk ls /uploads -z localhost:9983 ==> Command to show the znode "/uploads/data" exists |
|
Submitted a PR for 4 against your branch. Will work on 6 and 7 tonight. |
SOLR-17359:Fix for 'solr start -e cloud' example on Windows
|
Submitted PR #8 for issues 6 and 7. |
SOLR-17359: Resolve recursive option (-r) failure (eg: in 'zk cp -r' and 'zk rm -r')
Recurse is never provided as a "false", and we want these to be simpler, so let's make the existence of -r as default to apply recursion true...!
|
@rahulgoswami thanks for your work, I was able to take what you did and find a really nice refactoring on the |
|
Ok, so 6.iii) is a non-issue. The zk destination was missing a trailing '/' and the zk cp command works fine for a file once the destination ends in a '/' to indicate a parent znode. Although in traditional filesystems I would expect the command to work if the destination is a directory despite missing a file separator at the end, in case of Zk getting this to work might be tricky. Given the fact that there is no concept of file and directory in Zookeeper, and everything is just a znode, I could not find a reliable check to establish whether an empty znode destination should be classified as a "file" or "directory". This is where the trailing '/' helps clarify things imo. |
|
The only zk commands in the "solr zk --help" list that I have not been able to test are "zk linkconfig" and "zk updateacls". I am not sure how these are supposed to work/how to test them. The current documentation doesn't list the usage either (https://solr.apache.org/guide/solr/latest/deployment-guide/solr-control-script-reference.html#zookeeper-operations). |
|
Minor hiccup with solr zk --help command. "downconfig" is being displayed twice. Submitted a tiny PR #9. |
SOLR-17359: Typo: zk upconfig usage says 'downconfig'
This edit points to the weakness of having hard coded usage text!
Do you have any thoughts on ref guide text or maybe expanding the text in |
Thanks! I also saw that the getUsage needed updating, so I did that. This apporach of hard coding getUsage text is rather brittle and we really need to improve how things work underlying. (getUsage() is being overriden because we need |
Should all this content someday just go in this referenced page? we kind of duplicate!
|
@rahulgoswami I updated the ref guide docs to cross link to an EXISTING page for the upconfig/downconfig etc commands... Someday we'll reorg the ref guide page to make the solrcli easier to understand! |
|
I believe the one failing unit test doesn't have anything to do with this PR... This is ready for review and merging for 9.8! |
* Move Zk Arg parsing into Java Code from the bin/solr shell and bin/solr.cmd command scripts, making testing life easier on Windows. * Improved the output of -h to make usage information look nicer * restructure control flow so that we don't check for each script command by name and THEN call the java code, instead only look for the three exceptions, start, stop, restart. * Resolve Executing a command incorrectly gives a message which is not clear. Eg: >solr zk ls --------- Co-authored-by: Rahul Goswami <[email protected]> (cherry picked from commit 463e093)
https://issues.apache.org/jira/browse/SOLR-17359
Description
Migrate arg parsing for ZK commands into Java code
Solution
Had to add a ZkToolHelp to handle some of the global help output.
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:
mainbranch../gradlew check.