[Mailman-Developers] [GSoC 2014] Mailman CLI Project

[Rajeev S]

On Friday 09 May 2014 11:12 PM, Stephen J. Turnbull wrote:
> I would like to post this to Mailman-Developers, with your permission.
>
> Rajeev S writes:
>
>   > Hi Steve,Can I go forward with the current design?
>
> Looking at the code you've produced so far[1] I can say it's clean and
> (mostly) Pythonic, easy to read, making good use of packaged modules
> (and good choices of the ones to use).
>
> But to be honest, you haven't really *presented* a design yet.  A file
> tree as in your blog[2] isn't a design, and the descriptions of
> commands really amount to a set of example use cases, but hardly
> complete.  The design is implicit in patterns in the examples, and I
> have a feeling that it isn't yet coherent in your mind.
I only meant to expose my approach towards the project. Now, at least you
know how I have planned to approach the task at hand.
> So you can go forward, but if you continue to write code "as you go
> along", I think every couple of weeks we're going to have to refactor.
> That's OK if that's what you want (as you say in your blog
> "discussions over a piece of code would be more effective than just
> talk").  The alternative is to systematically explore the space of
> command + argument + option combinations, and try to come up with a
> language that compactly expresses the most common operations.  I
> recommend this approach if it works for you, because various options
> are starting to proliferate, and I would rather we keep them to a
> minimum so that users don't need to remember them all or be
> continuously consulting the documentation.  It may *not* work for you,
> and that's OK, we can go with the refactor-until-happy strategy then.
I too would prefer to go by the systematic approach. I felt the best way to
express my thoughts was to implement some part of it. I am refactoring
too much already.
> A couple of examples of the kind of things I think it would be useful
> to think about in advance:
>
> (1) You already have --domainname and --listname options, and of
>      course the pattern suggests --username for sure, and maybe there
>      will be additional --foonames.  I wonder if it wouldn't be better
>      to have a single --name option so that instead of
>
>      mmclient list create --domainname list.example.com --listname foo
>
>      the user types
>
>      mmclient list create --name foo --in-domain list.example.com
>
>      or
>
>      mmclient list create --namefoo at list.example.com
I doubt the usability of a common option `name`. As per the above 
snippet, you have
used a `in-domain` to specify the domain name. There are many such
instances where you would have to use more than one `name`, for instance
adding moderators for a list. In such cases, options like `domainname`, 
`listname`
and `username` would be more intuitive than using options like `in-domain`.

Further, the option `name` is ambiguous for a `user`.

>      or even
>
>      mmclient list createfoo at list.example.com
>
>      Note that the last is "Pythonic" in the sense that your
>      ArgumentParser will check for the right number of arguments.
This is a good way to do it, a more pythonic way,as mentioned.

Imposing a fair enough rule like the list must be specified only in
`list at domain` format can reduce a number of arguments in a *lot*
of commands.

By using a third (optional) positional argument that specifies
the `name` of the instance being managed, the commands look
more cleaner.

However, some commands still need more names to appear on the command
which I prefer *not* to replace by a single `--name`. The commands are 
mentioned below.

> (2) Should the scope (in [domain, list, user]) come first as in your
>      current design, "mmclient list create", or should the action come
>      first as in English, "mmclient create list"?  I think the latter
>      will be easier for non-technical admins to get used to, but it may
>      not be -- we need a list of "things we want to say" to decide that.
Commands that makes sense in English would suit most situations,
in fact, all situations in current list of use cases.

As the first step of the systematic design process, I present the list of
things we would want to say and how the command must look like and
what possible arguments it should accept.

I have modified the CLI to use English like commands and hence will
use them hereafter.

*list*

The command lists the entities and should be available for users,mailing 
lists and domains.

mmclient list list [list at domain.org] [-v for verbose]
mmclient list user [list at domain.org] [-v for verbose] [--role ROLE]
mmclient list domain [domain.org] [-v for verbose]

The ROLE would be any of member,owner or moderator.

*create*

Used to create an instance and should be available for domains, lists 
and users.

mmclient create domain domain.org [--contact CONTACT_EMAIL]
mmclient create list list at domain.org
mmclient create user foo at bar.com --name DISPLAY_NAME --password PASSWORD

*delete*

Removes a domain, list or user.

mmclient delete domain domain.org
mmclient delete list list at domain.org
mmclient delete user foo at bar.com

*Role management*

User roles can be managed by two actions, addrole and
removerole, rather than 6 separate actions for subscribe, unsubscribe,
addowner, addmoderator, removeowner, removemoderator

mmclient addrole user foo at bar.com --list list at domain.org --role 
<member,owner,moderator>
mmclient removerole user foo at bar.com --list list at domain.org --role 
<member,owner,moderator>

The above commands are an argument longer than the commands like

mmclient addmoderator list list at domain.org --user foo at bar.com

but I feel former approach better, as it looks better and reduces the number
of commands to learn/remember.

*Preferences/settings*

Settings would form a new `scope`, commands for which can be implemented
in a fairly straightforward way.

mmclient set setting <setting name> --value <value>

*Edit (user)*

Changing user credentials like email, password and display name

mmclient edit user foo at bar.com --email foo1 at bar.com
mmclient edit user foo at bar.com --name foo
mmclient edit user foo at bar.com --password bar

A common method that will support multiple arguments together
can be built for this action.
> I feel that it's likely to be quite annoying to refactor a ton of code
> if we decide to change those aspects of the design.  That's why I feel
> it's better to do a more detailed design now.
>
> A few unconnected comments on various aspects:
>
> - In your code you have some short variable names (d, l).  This is
>    mildly un-Pythonic.  Try reading diffs (eg, cmdparser.py in r52) and
>    I think you'll see.  If you don't see right off, I can explain
>    precisely why I stumbled in a couple of places, but I'd rather let
>    you see it for yourself.
Fixed.
> - Short names for long options like --ll are a definite no-no.  They
>    make the code harder to understand, and on the command line they are
>    arcane.  We need to cater to users who may not be so flexible.
I have replaced --ll with -v and --verbose.
> - The change you make in r52 is interesting.  I'm a little leery of
>    the use of getattr(), it's ugly, but that's probably easily fixed.
> - Once you have the parsed command, you look up the command and then
>    apply it to arguments retrieved from your ArgumentParser instance.
>    *These lookups can fail*, and you need to catch them (for
>    interactive shell mode).  I don't think ArgumentParser will check
>    that all syntactically required arguments are present, so you need
>    to do it yourself.  Think carefully about this -- you may want to
>    revise or even revert the change in r52 because it implies doing
>    this argument checking in each action rather than in the command
>    parser.
Why not leave the argument verification to the action method itself? I 
feel that
it's better to leave it there as it would make future addition of 
actions easier.

All the methods in the current version have a initial argument checking 
part. The
current version already `prints` error messages if required arguments 
are absent.

I will be replacing it with an `exception` so that it would be easier to 
integrate
it with the interactive shell.

Also argparse returns a dictionary with keys for *all* the possible 
options, whether or
not they are specified by the user. If the value of option is not 
specified by the user,
then the value of corresponding key would be `None`.

However, extra arguments are ignored without warning.
> - For detailed listings you add header in get_listing.  But I think
>    you should have a no-header option -- if you ever want to pipe that
>    output to a script the header is annoying.  My instinct was to move
>    it out of that function, but I guess you have to do it there to get
>    columnization right.
>
Adding --no-header requires just a trivial change.Will do.
> Some nitpicking about names:
>
>
> - "Operate" is pretty general, the conventional word for managing
>    lists of things is "Manage".
>
> - "Long list" is ambiguous ("one 'long list'" vs. a "paged list", for
>    example).  "Detailed" (your word) or maybe "verbose" (conventional)
>    is better.
>
> - I prefer the word "scope" for [domain, list, user] rather than
>    "type" or "instance".
Fixed.
> - Having "list" be both the name of a scope and the name of a command
>    in the same ArgumentParser means that you need to check syntax
>    (order of the words), not just presence of the words, to be sure the
>    user has given all the required arguments.  If the scope names and
>    command names are disjoint, then you could even let the user choose
>    whether to type "create list" or "list create".  (That may not be a
>    great idea.)
The so called positional arguments are parsed in the order that
they are listed in the initialize_options method. Hence the
order checking is performed by the argparse itself.
`create domain` is not same as `domain create`.

The presence of all required arguments is verified at
the method corresponding to an action.
> Footnotes:
> [1]http://bazaar.launchpad.net/~rajeevs1992/mailman.client/mailmancli/changes/
>
> [2]http://myfossblog.blogspot.in/2014/05/lets-talk-over-cup-of-code.html
>
>
In addition, I will be partitioning the commands/arguments to groups 
based on the
entity, by making use of the argparse sub-parsers. This would enhance 
the readability
of the help strings.

TODO list from this mail:

- no-header option for list
- positional argument for instance name
- exceptions on argument lookup failure
   - printing error messages on lookup failure
- partitioning arguments using sub-parsers

All parts mentioned fixed are fixed and will form r53, after some more 
cleaning, like the docs,
probably by tomorrow.The items mentioned in TODO lists will
be built once the design process is complete.