Re: [PATCH 00 of 17 v3] Introduction of kallithea-cli command

2018-10-23 Thread Thomas De Schampheleire
,
El lun., 22 oct. 2018 a las 1:11, Mads Kiilerich
() escribió:
>
> On 10/18/2018 10:49 PM, Thomas De Schampheleire wrote:
> > This series introduces a kallithea-cli command to replace our custom gearbox
> > commands.
> > At the end of this series, the help text will show:
> >
> >
> > Usage: kallithea-cli [OPTIONS] COMMAND [ARGS]...
> >
> >Various commands to set up a Kallithea instance.
> >
> > Options:
> >--help  Show this message and exit.
> >
> > Commands:
> >cache-cleanup-keys  Clean up existing cache keys.
> >cache-show-keys Show existing cache keys with their status.
>
> These two commands are really not so much about some kind of "keys".
> They are about database entries that indicate that some worker process
> has a repo cached in memory, so the cache can be invalidated and flushed
> when necessary. It would thus perhaps be better to name these commands
> so they are grouped with other repo commands, like:
>
> repo-cache-cleanup   Flush in-memory repository caches.
> repo-cache-showShow current in-memory repository caches.
>
> >celery-run  Start Celery worker(s) for asynchronous tasks.
> >config-create   Create a new configuration file.
> >db-create   Initialize the database.
> >extensions-create   Write template file for extending Kallithea in 
> > Python.
> >front-end-createCreate the front-end.
>
> The "create" naming consistency is generally nice, but for this one,
> "build" seems much more descriptive and correct.
>
> >iis-install Install into IIS using isapi-wsgi.
> >index-createCreate or update full text search index
> >ishell-run  Interactive shell for Kallithea.
> >repo-purge-deleted  Purge backups of deleted repositories.
> >repo-scan   Scan filesystem for repositories.
> >repo-update-cache   Update repo cache after external modification.
>
> This one is perhaps better described as: Update database cache of latest
> repository change.

Regarding the command names, we now have suggested:

repo-cache-cleanup
repo-cache-show
repo-update-cache

There is some inconsistency here that needs to be solved IMO.
The 'update-cache' not only handles the latest repository change, it
also invalidates the database cache.

For repo-cache-cleanup/show: under which circumstances would a user
execute any of these commands? Are these commands actually necessary?
Or can they be combined with repo-update-cache, possibly renamed?

>
> > Some notes:
> > - In v1 and v2, each topic was a separate word in the CLI, e.g. 'repo
> >update-cache' rather than 'repo-update-cache'. There were two problems
> >with that approach:
> >1. to know which commands are available, you'd use 'kallithea-cli --help'
> >   to discover the topics, then 'kallithea-cli topic --help', and then
> >   you'd still need another 'kallithea-cli topic command --help' to
> >   get a list of the actual options.
>
> Hmm. I see. Good point. Annoying how it seems to force us to use a
> not-so-elegant solution with the "object" and the "method" tied together
> in the command name. But it also gives a more flat namespace that is
> more explorable ...
>
> In a way, it could perhaps be more elegant with more specific top level
> command names, something like
>kallithea-repo update-cache my.ini foorepo barrepo
> but that would have some of the same problems ... and perhaps give too
> many commands, which might be a problem or a solution ...

It's technically possible, sure. But we'd have quite a large amount of
top-level commands, I'd say a bit too much to make it obvious.
But see also further in this mail...

>
> Also related to that: when thinking about a consistent solution, we
> should perhaps also consider where the existing kallithea_api.py
> kallithea_backup.py  kallithea_gist.py could fit in.

Of these, I only used kallithea_api.py.

With the current scheme, we could have following?
kallithea-cli api-call
kallithea-cli gist-create
kallithea-cli backup-create

>
> >2. some command-lines would become quite long, and distinction between
> >elements of the command itself, and its arguments, were hard to see. For
> >example, to update repository caches:
> >   kallithea-cli repo update-cache my.ini foorepo barrepo
> >   vs.
> >   kallithea-cli repo-update-cache my.ini foorepo barrepo
>
> Also, looking at the examples (both commands in this patch series an
> examples from other tools), it seems less elegant when commands have
> different "positional parameters", like how parameter 1 is the .ini file
> and parameter 2 and up are repository names. I kind of like how the
> gearbox solution consistently use -c for these config files. The general
> rule for command line design might be that un-named parameters only
> should be used to name the main things we operate on.

I was following the guideline that mandatory arguments should not be
passed via options. Options should be optional.

Re: [PATCH 00 of 17 v3] Introduction of kallithea-cli command

2018-10-21 Thread Mads Kiilerich

On 10/18/2018 10:49 PM, Thomas De Schampheleire wrote:

This series introduces a kallithea-cli command to replace our custom gearbox
commands.
At the end of this series, the help text will show:


Usage: kallithea-cli [OPTIONS] COMMAND [ARGS]...

   Various commands to set up a Kallithea instance.

Options:
   --help  Show this message and exit.

Commands:
   cache-cleanup-keys  Clean up existing cache keys.
   cache-show-keys Show existing cache keys with their status.


These two commands are really not so much about some kind of "keys". 
They are about database entries that indicate that some worker process 
has a repo cached in memory, so the cache can be invalidated and flushed 
when necessary. It would thus perhaps be better to name these commands 
so they are grouped with other repo commands, like:


repo-cache-cleanup   Flush in-memory repository caches.
repo-cache-show    Show current in-memory repository caches.


   celery-run  Start Celery worker(s) for asynchronous tasks.
   config-create   Create a new configuration file.
   db-create   Initialize the database.
   extensions-create   Write template file for extending Kallithea in Python.
   front-end-createCreate the front-end.


The "create" naming consistency is generally nice, but for this one, 
"build" seems much more descriptive and correct.



   iis-install Install into IIS using isapi-wsgi.
   index-createCreate or update full text search index
   ishell-run  Interactive shell for Kallithea.
   repo-purge-deleted  Purge backups of deleted repositories.
   repo-scan   Scan filesystem for repositories.
   repo-update-cache   Update repo cache after external modification.


This one is perhaps better described as: Update database cache of latest 
repository change.



Some notes:
- In v1 and v2, each topic was a separate word in the CLI, e.g. 'repo
   update-cache' rather than 'repo-update-cache'. There were two problems
   with that approach:
   1. to know which commands are available, you'd use 'kallithea-cli --help'
  to discover the topics, then 'kallithea-cli topic --help', and then
  you'd still need another 'kallithea-cli topic command --help' to
  get a list of the actual options.


Hmm. I see. Good point. Annoying how it seems to force us to use a 
not-so-elegant solution with the "object" and the "method" tied together 
in the command name. But it also gives a more flat namespace that is 
more explorable ...


In a way, it could perhaps be more elegant with more specific top level 
command names, something like

  kallithea-repo update-cache my.ini foorepo barrepo
but that would have some of the same problems ... and perhaps give too 
many commands, which might be a problem or a solution ...


Also related to that: when thinking about a consistent solution, we 
should perhaps also consider where the existing kallithea_api.py 
kallithea_backup.py  kallithea_gist.py could fit in.



   2. some command-lines would become quite long, and distinction between
   elements of the command itself, and its arguments, were hard to see. For
   example, to update repository caches:
  kallithea-cli repo update-cache my.ini foorepo barrepo
  vs.
  kallithea-cli repo-update-cache my.ini foorepo barrepo


Also, looking at the examples (both commands in this patch series an 
examples from other tools), it seems less elegant when commands have 
different "positional parameters", like how parameter 1 is the .ini file 
and parameter 2 and up are repository names. I kind of like how the 
gearbox solution consistently use -c for these config files. The general 
rule for command line design might be that un-named parameters only 
should be used to name the main things we operate on.



- We need to find a better place for template.ini.mako which is now the sole
   inhabitant of kallithea/lib/paster_commands/.


kallithea/templates/ is mostly html templates for the front-end, but we 
also have kallithea/templates/email_templates/ , so perhaps this one 
could be placed as something like kallithea/templates/config.ini .



s ...


With these nice experiments and overviews, it is interesting to take a 
step back and look at them and other complex and more or less elegant 
commands with sub-commands. Here are some I know about:
https://docs.docker.com/engine/reference/commandline/cli/ 
https://docs.docker.com/engine/reference/commandline/docker/

https://docs.ansible.com/ansible/2.4/command_line_tools.html
https://dnf.readthedocs.io/en/latest/command_ref.html
https://linux.die.net/man/1/fedpkg
https://www.mercurial-scm.org/doc/hg.1.html
http://man7.org/linux/man-pages/man1/git.1.html
https://www.openssl.org/docs/man1.1.1/man1/openssl.html

One thing I conclude is that it indeed only is common to use one level 
of sub commands. But it varies whether sub-sub commands are implemented 
by having verbose sub command names (seems quite common for example in 
git) or using