Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh merged PR #2820: URL: https://github.com/apache/solr/pull/2820 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2452141047 > Looks good to me, there were a lot of changes and we definitely have a few more places we can reduce a few more lines. But that's for another PR. > > @epugh It seems that the changes from #2816 that were already merged are not included here. Is this a problem? Just pushed up the latest updates from `main` to pick up #2816. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2451761906 I'd love to get this merged today if anyone can give it a quick pass. I am sure there is more we can do, but would like to unblock some other PR's. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824808865
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did you mean --version?\n");
+ exit(1);
Review Comment:
I think at that point, I might suggest we introduce a `bin/solr inspect`
command.. that could be then used to do lots of interesting things...
so we don't end up with `bin/solr --version` and `bin/solr version -s
http://mycorp.dev.solr:234525` ;-)
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
malliaridis commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824945975
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did you mean --version?\n");
+ exit(1);
Review Comment:
> I think at that point, I might suggest we introduce a bin/solr inspect
command.. that could be then used to do lots of interesting things...
I like this idea, especially if it comes to multiple Solr clusters /
instances, this would probably be helpful. But I think this is more of a
command we may introduce in the future.
If I remember correct, we also discussed the command `solr version` and
decided to drop support, rather than implement functionality under this command
for supporting urls as well.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824234733
##
solr/core/src/java/org/apache/solr/cli/ApiTool.java:
##
@@ -57,37 +56,19 @@ public List getOptions() {
.longOpt("solr-url")
.argName("URL")
.hasArg()
-.required(false) // swap back to required when we eliminate
deprecated option
-.desc("Send a GET request to a Solr API endpoint.")
-.build(),
-Option.builder("get")
-.longOpt("get")
-.deprecated(
-DeprecatedAttributes.builder()
-.setForRemoval(true)
-.setSince("9.7")
-.setDescription("Use --solr-url instead")
-.get())
-.argName("URL")
-.hasArg()
-.required(false)
+.required(true)
.desc("Send a GET request to a Solr API endpoint.")
.build(),
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
-String response = null;
-String getUrl =
-cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") :
cli.getOptionValue("get");
-if (getUrl != null) {
- response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
-}
-if (response != null) {
- // pretty-print the response to stdout
- echo(response);
-}
+String getUrl = cli.getOptionValue("solr-url");
Review Comment:
I flipped the "required" back to true, so it can't be null!
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2450346488 I also anticipate that post this merge and some of the other inflight PR's, that we will want to reach out and ask the community for another round of testing on LInux and windows ;-) -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2450344779 I **think** I am done and ready for more review. I clearly was NOT done when I thought I was the first time!!! I love that while we added 176 lines we remove 1092 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824809265
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did you mean --version?\n");
+ exit(1);
Review Comment:
I went ahead and took your suggestion @malliaridis
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820:
URL: https://github.com/apache/solr/pull/2820#issuecomment-2450147305
> @janhoy in commit
[2f3c0ef](https://github.com/apache/solr/commit/2f3c0ef959b6a1f921217ef30da5347ec17a5b5f)
I removed the passing of the `-j` (Jetty options) into the RunExampleTool
because it was an option that was ignored. You added the line in the bin/solr
so wanted to flag this to you in case you think we need to someone add this
capability to RunExampleTool!
@janhoy never mind, turns out regardless of running Solr in foreground or
not, we pass in the settings via `start_solr "$FG" "${ADDITIONAL_CMD_OPTS:-}"
"${ADDITIONAL_JETTY_CONFIG:-}"`. So the removed line was never actually used!
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2450138855 @janhoy in commit 2f3c0ef959b6a1f921217ef30da5347ec17a5b5f I removed the passing of the `-j` (Jetty options) into the RunExampleTool because it was an option that was ignored. You added the line in the bin/solr so wanted to flag this to you in case you think we need to someone add this capability to RunExampleTool! -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2449846136 I am leaving `SolrCLI#getOptionWithDeprecatedAndDefault` but adding some docs about what it's for, since I suspect we will have another deprecation soon! -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
janhoy commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824575199
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did you mean --version?\n");
+ exit(1);
Review Comment:
I feel the `--version` flag should report the version of the local Solr CLI
program, but if we implement a `version` cmd that accepts `--solr-url` then it
should report the version of the remote solr host?
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824243229
##
solr/packaging/test/test_snapshots.bats:
##
@@ -59,15 +59,15 @@ teardown() {
@test "snapshot list" {
solr snapshot-create -c films --snapshot-name snapshot3 --solr-url
http://localhost:${SOLR_PORT}
- run solr snapshot-list -c films -url http://localhost:${SOLR_PORT}/solr
+ run solr snapshot-list -c films -s http://localhost:${SOLR_PORT}/solr
Review Comment:
i don't know if in a future solr 11 that we actually just refuse to work
with `/solr` ;-).
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2449861488 for "AuthTool uses flags with Boolean arguments, should be deprecated and changed before 10" we should make another ticket. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on PR #2820: URL: https://github.com/apache/solr/pull/2820#issuecomment-2449839719 I hope in the future `solr.cmd` doesn't have helper methods like `parse_config_args` -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824235207
##
solr/core/src/java/org/apache/solr/cli/ApiTool.java:
##
@@ -57,37 +56,19 @@ public List getOptions() {
.longOpt("solr-url")
.argName("URL")
.hasArg()
-.required(false) // swap back to required when we eliminate
deprecated option
-.desc("Send a GET request to a Solr API endpoint.")
-.build(),
-Option.builder("get")
-.longOpt("get")
-.deprecated(
-DeprecatedAttributes.builder()
-.setForRemoval(true)
-.setSince("9.7")
-.setDescription("Use --solr-url instead")
-.get())
-.argName("URL")
-.hasArg()
-.required(false)
+.required(true)
.desc("Send a GET request to a Solr API endpoint.")
.build(),
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
-String response = null;
-String getUrl =
-cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") :
cli.getOptionValue("get");
-if (getUrl != null) {
- response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
-}
-if (response != null) {
- // pretty-print the response to stdout
- echo(response);
-}
+String getUrl = cli.getOptionValue("solr-url");
+String response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
+
+// pretty-print the response to stdout
+echo(response);
Review Comment:
or a bats test!
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824242674
##
solr/packaging/test/test_snapshots.bats:
##
@@ -59,15 +59,15 @@ teardown() {
@test "snapshot list" {
solr snapshot-create -c films --snapshot-name snapshot3 --solr-url
http://localhost:${SOLR_PORT}
- run solr snapshot-list -c films -url http://localhost:${SOLR_PORT}/solr
+ run solr snapshot-list -c films -s http://localhost:${SOLR_PORT}/solr
Review Comment:
i checked hte other bats tests, and yes, we normalize away the /solr. I am
going to add a comment though that it's here to confirm that we continue to
handle trailing `/solr` properly.
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824240009
##
solr/packaging/test/test_snapshots.bats:
##
@@ -59,15 +59,15 @@ teardown() {
@test "snapshot list" {
solr snapshot-create -c films --snapshot-name snapshot3 --solr-url
http://localhost:${SOLR_PORT}
- run solr snapshot-list -c films -url http://localhost:${SOLR_PORT}/solr
+ run solr snapshot-list -c films -s http://localhost:${SOLR_PORT}/solr
Review Comment:
good questin, and I wonder if we normalize anyway...
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824239234
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did you mean --version?\n");
+ exit(1);
Review Comment:
so this is a bit of a werid one, which is right now `bin/solr version` works
the same as `bin/solr --version`...Should we keep the check and just give
the same stock error message that we do if you do `bin/solr fakecommand`, which
would be "fakecommand is not a valid command!"???
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
epugh commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1824218403
##
solr/core/src/java/org/apache/solr/cli/ApiTool.java:
##
@@ -57,37 +56,19 @@ public List getOptions() {
.longOpt("solr-url")
.argName("URL")
.hasArg()
-.required(false) // swap back to required when we eliminate
deprecated option
-.desc("Send a GET request to a Solr API endpoint.")
-.build(),
-Option.builder("get")
-.longOpt("get")
-.deprecated(
-DeprecatedAttributes.builder()
-.setForRemoval(true)
-.setSince("9.7")
-.setDescription("Use --solr-url instead")
-.get())
-.argName("URL")
-.hasArg()
-.required(false)
+.required(true)
.desc("Send a GET request to a Solr API endpoint.")
.build(),
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
-String response = null;
-String getUrl =
-cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") :
cli.getOptionValue("get");
-if (getUrl != null) {
- response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
-}
-if (response != null) {
- // pretty-print the response to stdout
- echo(response);
-}
+String getUrl = cli.getOptionValue("solr-url");
+String response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
+
+// pretty-print the response to stdout
+echo(response);
Review Comment:
i don't think it can ever be mull... it would be an error message etc.
If there is an scenario where it *coud* be null, we should add a unit test!
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17352: Remove deprecated options from CLI [solr]
malliaridis commented on code in PR #2820:
URL: https://github.com/apache/solr/pull/2820#discussion_r1823372644
##
solr/core/src/java/org/apache/solr/cli/ApiTool.java:
##
@@ -57,37 +56,19 @@ public List getOptions() {
.longOpt("solr-url")
.argName("URL")
.hasArg()
-.required(false) // swap back to required when we eliminate
deprecated option
-.desc("Send a GET request to a Solr API endpoint.")
-.build(),
-Option.builder("get")
-.longOpt("get")
-.deprecated(
-DeprecatedAttributes.builder()
-.setForRemoval(true)
-.setSince("9.7")
-.setDescription("Use --solr-url instead")
-.get())
-.argName("URL")
-.hasArg()
-.required(false)
+.required(true)
.desc("Send a GET request to a Solr API endpoint.")
.build(),
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
-String response = null;
-String getUrl =
-cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") :
cli.getOptionValue("get");
-if (getUrl != null) {
- response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
-}
-if (response != null) {
- // pretty-print the response to stdout
- echo(response);
-}
+String getUrl = cli.getOptionValue("solr-url");
+String response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
+
+// pretty-print the response to stdout
+echo(response);
Review Comment:
What if `response` is `null`, should we print it anyway?
##
solr/packaging/test/test_snapshots.bats:
##
@@ -59,15 +59,15 @@ teardown() {
@test "snapshot list" {
solr snapshot-create -c films --snapshot-name snapshot3 --solr-url
http://localhost:${SOLR_PORT}
- run solr snapshot-list -c films -url http://localhost:${SOLR_PORT}/solr
+ run solr snapshot-list -c films -s http://localhost:${SOLR_PORT}/solr
Review Comment:
This may be a bit out of scope, but should `/solr` here be supported /
allowed? Because in my opinion the value passed to `-s` should be the same in
all commands, and right now we mix them. Additionally, `/solr` smells like v1
API, which is not future-proof probably.
##
solr/core/src/java/org/apache/solr/cli/ApiTool.java:
##
@@ -57,37 +56,19 @@ public List getOptions() {
.longOpt("solr-url")
.argName("URL")
.hasArg()
-.required(false) // swap back to required when we eliminate
deprecated option
-.desc("Send a GET request to a Solr API endpoint.")
-.build(),
-Option.builder("get")
-.longOpt("get")
-.deprecated(
-DeprecatedAttributes.builder()
-.setForRemoval(true)
-.setSince("9.7")
-.setDescription("Use --solr-url instead")
-.get())
-.argName("URL")
-.hasArg()
-.required(false)
+.required(true)
.desc("Send a GET request to a Solr API endpoint.")
.build(),
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
-String response = null;
-String getUrl =
-cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") :
cli.getOptionValue("get");
-if (getUrl != null) {
- response = callGet(getUrl,
cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
-}
-if (response != null) {
- // pretty-print the response to stdout
- echo(response);
-}
+String getUrl = cli.getOptionValue("solr-url");
Review Comment:
getUrl may be `null` here and throw `NullPointerException` further down.
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -215,18 +150,16 @@ public static void exit(int exitStatus) {
public static void main(String[] args) throws Exception {
final boolean hasNoCommand =
args == null || args.length == 0 || args[0] == null ||
args[0].trim().length() == 0;
-final boolean isHelpCommand =
-!hasNoCommand && Arrays.asList("-h", "--help", "/?").contains(args[0]);
+final boolean isHelpCommand = !hasNoCommand && Arrays.asList("-h",
"--help").contains(args[0]);
if (hasNoCommand || isHelpCommand) {
printHelp();
exit(1);
}
-if (Arrays.asList("-version", "version").contains(args[0])) {
- // select the version tool to be run
- CLIO.out("Deprecated operation as of 9.8. Please use bin/solr
--version.");
- args = new String[] {"version"};
+if (args[0].equalsIgnoreCase("version")) {
+ CLIO.out("version is not a valid command! Did
