-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewrite CLI options handling and output #5
Conversation
* Use argparse for option handling * Remove global variables and put them under an argparse namespace
* Replace 'print' statements with the appropriate log calls * Replace sys.stderr calls with the appropriate log calls * Remove global variable HUPTIME_DEBUG
Thanks, this is great! There seem to be a couple of small issues at the moment, though:
I assume this is because we need to always cast ARGS.unlink as well?
This was weird to start with. I think now HUPTIME_MULTI will contain a number, whereas it needs to contain "true". The python script itself is the only thing that uses the N to spawn the appropriate number of child processes.
This is a personal preference, but I think there's a good reason. Because I expect the usage of the tool to be automated more than interactive (i.e. it'll end up in init scripts or configuration files), I'd rather force the long form arguments. In my experience, using the long form arguments in scripts is best practice simply because it makes things more readable and maintainable for others (just compare "huptime --unlink /var/run/myprog.pid --revive --fork {...}" to "huptime -u /var/run/myprog.pid -r -f {...}"). |
Will eliminate support for short arguments, I do share your preference as well just thought it might appeal to a wider audience that way. (except for --debug/-d perhaps? since this is more interactive?) RE: --unlink broken RE: Behavior of --multi |
TIL: Leading underscores followed by a capital letter is actually a reserved identifier. This replaces the header guards with a non-reserved identifier.
Fixed the above (--debug/-d stays, let me know if you do not want that), rebased on master. |
This will automatically clean-up all the child processes spawned by --multi whenever the parent dies. This is much more useful for integration with a supervisor (like upstart) because you can easily kill the collection without needing to use killall or --stop.
I added a commit to touch it up a bit (print help w/ no command, etc.) and made #16. Anything missing from there? |
#16 looks good |
Rolled in #16 (where uh, it continues to languish in limbo. Sorry.) |
I needed more stable and extensible option handling so argparse replaces the custom option handling code. And I also replaced the various
print
andsys.stderr.write
calls with the logging module.