Move Clamp away from Agent, Remove Subcommands and Clean CLI #3872
Conversation
|
This breaks my PR on agent for default filter_workers :-) |
| :attribute_name => :log_file | ||
|
|
||
| # Old support for the '-v' flag' | ||
| option "-v", :flag, |
There was a problem hiding this comment.
do we need this still? we can remove this since we support --debug. Than we can get rid of -V and call it -v for version like all other systems do. Thoughts?
There was a problem hiding this comment.
I'm open to that, though it may be surprising to users. If anything, I'd prefer that -v went away and did not change to report the version - mostly to avoid surprising users.
There was a problem hiding this comment.
Historically -v has been used to increase log verbosity, so its mostly an advanced user functionality. I would argue that replacing it with version output wouldn't surprise them. Maybe they have already migrated to --debug. All in all -v is a no-side-affect action, and folks wouldn't have used this in scripts so its less dangerous :)
|
@guyboertje I'll likely lose the race on master, so I'll adapt this, don't worry :D |
efc5923 to
61b1ef2
Compare
12c2c13 to
281edc1
Compare
| @@ -30,95 +85,245 @@ def main(args) | |||
| # Print a warning to STDERR for bad java versions | |||
| LogStash::Util::JavaVersion.warn_on_bad_java_version | |||
|
|
|||
| Stud::untrap("INT", @startup_interruption_trap) | |||
| if version? | |||
There was a problem hiding this comment.
It would be awesome if this function was broken up into smaller ones
show_version
configure
return start_shell(@ruby_shell, binding) if @ruby_shell
check_config_present
...etc. would be a nice small way to clean this up. Not sure if you're the original author here or not. If it's just copy/paste don't worry about it.
|
On the whole this looks very good. Thanks for the work @jsvd I would have preferred that we not deprecate commands unless strictly necessary, but then again, times change. LGTM. |
|
@andrewvc I have updated this to ensure bwc with the bin/logstash [irb/pry] |
|
@jsvd awesome :) I think our users will appreciate it! 👍 |
| fail(I18n.t("logstash.agent.configuration.plugin_path_missing", :path => path)) unless File.directory?(path) | ||
| LogStash::Environment.add_plugin_path(path) | ||
| end | ||
| def shutdown_pipelines |
There was a problem hiding this comment.
Why do we have two shutdown methods? (shutdown and shutdown_pipelines)
The shutdown method and the hook was added to fix an issue with the test see
#4024
|
LGTM! really minor comments. |
d9368ca to
7093d5e
Compare
* move cli argument handling from agent to runner * add a short-help message * add interactive shell option * log fatal messages to terminal when logging to file * change docs:generate task to use bundle exec
|
João Duarte merged this into the following branches!
|
* move cli argument handling from agent to runner * add a short-help message * add interactive shell option * log fatal messages to terminal when logging to file * change docs:generate task to use bundle exec Fixes elastic#3872
Depends on jordansissel/ruby-cabin#37