SOLR-17469: CLI: Resolve -f flag conflicts #2740
Conversation
…-force-delete-config
-f means Force, and when I read what this does, by overwriting things, that seems to meet the definition of -f, so instead of changing the parameter, I tweaked the description.
github-actions
bot
added
documentation
|
@malliaridis would love your review! |
There was a problem hiding this comment.
Changes look very clean, thanks.
The foreground option that uses currently -f in bin/solr and bin/solr.cmd have not been updated. Are we planning to change that option as well?
And a few more hits I found for -f (config-file, SolrExporter) that should be updated:
- solr/solr-ref-guide/modules/deployment-guide/pages/monitoring-with-prometheus-and-grafana.adoc
- solr/prometheus-exporter/bin/solr-exporter.cmd
- solr/prometheus-exporter/bin/solr-exporter
solr/bin/install_solr_service.sh
Outdated
| @@ -46,7 +46,7 @@ print_usage() { | |||
| echo " -u User to own the Solr files and run the Solr process as; defaults to solr" | |||
| echo " This script will create the specified user account if it does not exist." | |||
| echo "" | |||
| echo " -f Upgrade Solr. Overwrite symlink and init script of previous installation." | |||
| echo " -f Force upgrade of Solr. Overwrite symlink and init script of previous installation." | |||
There was a problem hiding this comment.
Does -f stand for force in this context? Because I don't see any forcing action and the below message is printed in case of an error when -f is used:
/etc/systemd/system/$SOLR_SERVICE.service already exists! Perhaps Solr is already setup as a service on this host? To upgrade Solr use the -f option.
This seems a bit misleading and may require clarification.
There was a problem hiding this comment.
Maybe part of this is I don't want to touch the install service ;-). Do we have another bug? Because it appears that you tried to upgrade with a -f, it failed, and told you to use -f ????
There was a problem hiding this comment.
Yeah, I was thinking if it is easy enough to migrate the file to a Java class as well, but that's another topic. For now it's fine if we don't touch it and just be aware of its existence.
Do we have another bug? Because it appears that you tried to upgrade with a -f, it failed, and told you to use -f ????
I'm not sure how it should behave in case of -f and if it actually stands for upgrade, force or both. It looks like the action taken is a simple upgrade, but the param should be something like --upgrade, instead of -f. In that case the message would make sense. Failing to upgrade would tell the user to force the action with -f/--force.
There was a problem hiding this comment.
i am also somewhat thinking that maybe we don't touch install_solr_service.sh.. it's only run once, it has no tests, and I'm somewhat leery of taking it on. What if we finish all the flag conflicts in the bin/solr area, and then, in a seperate ticket, we go back and fix up install_solr_service.sh in a single JIRA?
Keep -f for forcing actions since it is often used in other tools for "force" as well. This means keep -f for force in bin/solr and RunExampleTool to allow starting Solr as root user and also for force deleting configuration directories in DeleteTool. Deprecate (9x) and remove (10.0) by replacing the "force-delete-config" (--force-delete-config, -forceDeleteConfig, --forceDeleteConfig, -f) in DeleteTool with the "force" option ( --force, -f). Deprecate (9x) and remove (10.0) -f from the "format" option in PostTool. Deprecate (9x) and remove (10.0) -f from the "config file" option (-f, --config-file, -config-file) in SolrExporter. (cherry picked from commit 08a17da)
https://issues.apache.org/jira/browse/SOLR-17469
Description
Keep -f for forcing actions since it is often used in other tools for "force" as well. This means keep -f for force in bin/solr and RunExampleTool to allow starting Solr as root user and also for force deleting configuration directories in DeleteTool.
Deprecate (9x) and remove (10.0) by replacing the "force-delete-config" (--force-delete-config, -forceDeleteConfig, --forceDeleteConfig, -f) in DeleteTool with the "force" option ( --force, -f).
Deprecate (9x) and remove (10.0) -f from the "format" option in PostTool.
Deprecate (9x) and remove (10.0) -f from the "config file" option (-f, --config-file, -config-file) in SolrExporter.
Deprecate (9x) and remove (10.0) -f from "upgrade solr" in install_solr_service.sh by replacing it with "–upgrade".
Solution
what it says
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.