Consider switching to argparse for nicer command line handling and help

manics commented 2 years ago

Proposed change

Switch from tornado.options to argparse (or traitlets, though argparse is easier for a small utility like this) for handling command line arguments

Alternative options

Do nothing

Who would use this feature?

Anyone wanting a nice help message. Currently it looks like this

Usage: /opt/conda/envs/test/bin/jupyterhub-idle-culler [OPTIONS]

Options:

  --help                           show this help information

/opt/conda/envs/test/lib/python3.9/site-packages/jupyterhub_idle_culler/__init__.py options:

  --api-page-size                  Number of users to request per page, when
                                   using JupyterHub 2.0's paginated user list
                                   API. Default: user the server-side default
                                   configured page size. (default 0)
  --concurrency                    Limit the number of concurrent requests made
                                   to the Hub.  Deleting a lot of users at the
                                   same time can slow down the Hub, so limit
                                   the number of API requests we have
                                   outstanding at any given time. (default 10)
  --cull-admin-users               Whether admin users should be culled (only
                                   if --cull-users=true). (default True)
  --cull-every                     The interval (in seconds) for checking for
                                   idle servers to cull. (default 0)
  --cull-users                     Cull users in addition to servers.  This is
                                   for use in temporary-user cases such as
                                   tmpnb. (default False)
  --internal-certs-location        The location of generated internal-ssl
                                   certificates (only needed with --ssl-
                                   enabled=true). (default internal-ssl)
  --max-age                        The maximum age (in seconds) of servers that
                                   should be culled even if they are active.
                                   (default 0)
  --remove-named-servers           Remove named servers in addition to stopping
                                   them.  This is useful for a BinderHub that
                                   uses authentication and named servers.
                                   (default False)
  --ssl-enabled                    Whether the Jupyter API endpoint has TLS
                                   enabled. (default False)
  --timeout                        The idle timeout (in seconds). (default 600)
  --url                            The JupyterHub API URL.

/opt/conda/envs/test/lib/python3.9/site-packages/tornado/log.py options:

  --log-file-max-size              max size of log files before rollover
                                   (default 100000000)
  --log-file-num-backups           number of log files to keep (default 10)
  --log-file-prefix=PATH           Path prefix for log files. Note that if you
                                   are running multiple tornado processes,
                                   log_file_prefix must be different for each
                                   of them (e.g. include the port number)
  --log-rotate-interval            The interval value of timed rotating
                                   (default 1)
  --log-rotate-mode                The mode of rotating files(time or size)
                                   (default size)
  --log-rotate-when                specify the type of TimedRotatingFileHandler
                                   interval other options:('S', 'M', 'H', 'D',
                                   'W0'-'W6') (default midnight)
  --log-to-stderr                  Send log output to stderr (colorized if
                                   possible). By default use stderr if
                                   --log_file_prefix is not set and no other
                                   logging is configured.
  --logging=debug|info|warning|error|none 
                                   Set the Python log level. If 'none', tornado
                                   won't touch the logging configuration.
                                   (default info)

Problems include:

The help message is interrupted by a path to a source file e.g. /opt/conda/envs/test/lib/python3.9/site-packages/jupyterhub_idle_culler/__init__.py
It's not clear to a new user how to use boolean options. E.g. can they be used as flags (--cull-users instead of --cull-users=true), what values can they take (is it just true/false, or are other values like 0/1 accepted)
It's not immediately obvious that = is required (--url http://localhost/ is invalid)
/opt/conda/envs/test/lib/python3.9/site-packages/tornado/log.py options are arguably irrelevant

(Optional): Suggest a solution

Use argparse instead. Boolean options would be implemented as choices=["true","false"] instead of a flag for compatibility.

Get rid of the logging options? This would be a breaking change though.

welcome[bot] commented 2 years ago

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively. welcome You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

consideRatio commented 1 year ago

I'm :+1: for a switch to argparse!

manics commented 1 year ago

One factor when choosing between argparse and traitlets is how we want to handle extensions and their configuration.

jupyterhub / jupyterhub-idle-culler