Re: [Mailman-Developers] [GSoC 2014] Mailman CLI Project
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@list.example.comI doubt the usability of a common option
name. As per the above snippet, you have used ain-domainto specify the domain name. There are many such instances where you would have to use more than onename, for instance adding moderators for a list. In such cases, options likedomainname,listnameandusernamewould be more intuitive than using options likein-domain.
Further, the option name is ambiguous for a user.
or even mmclient list createfoo@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@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@domain.org] [-v for verbose] mmclient list user [list@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@domain.org mmclient create user foo@bar.com --name DISPLAY_NAME --password PASSWORD
*delete*
Removes a domain, list or user.
mmclient delete domain domain.org mmclient delete list list@domain.org mmclient delete user foo@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@bar.com --list list@domain.org --role <member,owner,moderator> mmclient removerole user foo@bar.com --list list@domain.org --role <member,owner,moderator>
The above commands are an argument longer than the commands like
mmclient addmoderator list list@domain.org --user foo@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@bar.com --email foo1@bar.com mmclient edit user foo@bar.com --name foo mmclient edit user foo@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.
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
Adding --no-header requires just a trivial change.Will do.
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.
---------- Forwarded message ---------- From: "Rajeev S" <rajeevs1992@gmail.com> Date: May 11, 2014 10:38 PM Subject: Re: [Mailman-Developers] [GSoC 2014] Mailman CLI Project To: "Stephen J. Turnbull" <stephen@xemacs.org>, < mailman-developers@python.org>
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@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 evenmmclient list createfoo@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@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@domain.org] [-v for verbose] mmclient list user [list@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@domain.org mmclient create user foo@bar.com --name DISPLAY_NAME --password PASSWORD
*delete*
Removes a domain, list or user.
mmclient delete domain domain.org mmclient delete list list@domain.org mmclient delete user foo@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@bar.com --list list@domain.org --role <member,owner,moderator> mmclient removerole user foo@bar.com --list list@domain.org --role <member,owner,moderator>
The above commands are an argument longer than the commands like
mmclient addmoderator list list@domain.org --user foo@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@bar.com --email foo1@bar.com mmclient edit user foo@bar.com --name foo mmclient edit user foo@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.
[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
Footnotes: 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.
On May 11, 2014, at 10:37 PM, Rajeev S wrote:
I doubt the usability of a common option
name. As per the above snippet, you have used ain-domainto specify the domain name. There are many such instances where you would have to use more than onename, for instance adding moderators for a list. In such cases, options likedomainname,listnameandusernamewould be more intuitive than using options likein-domain.
You'll want to take a cue from the mailman create command, and as a general
rule, I think the shell-cli should be as similar as possible to the rest-cli.
I'm open to modifying the former in order to better align them, as long as we
have a consistent view of commands and options across both tools. Let's not
go down the accretion-without-design path of git. :)
-Barry
On May 11, 2014, at 10:37 PM, Rajeev S wrote:
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@domain.org] [-v for verbose] mmclient list user [list@domain.org] [-v for verbose] [--role ROLE] mmclient list domain [domain.org] [-v for verbose]
A better name might be show since the term "list" is so overloaded in this
context. Here's it's being used as a verb and a noun to refer to different
concepts, and I think that's confusing.
Also as a general rule, I think we want just one level of subcommand, so that
mmclient show --list would be the template. (That's open to discussion.)
*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
Yuck. Sorry but I'd like to discourage the use of made up or concatenated words. It's okay for some options to be multi-word separated by dashes, e.g. --affect-bar or --change-foo.
I don't have a good alternative atm.
*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>
E.g.
mmclient set --key <name> --value <value>
-Barry
Barry Warsaw writes:
Also as a general rule, I think we want just one level of subcommand, so that
mmclient show --listwould be the template. (That's open to discussion.)
I wonder about this in the context of argparse and the command line, because argparse makes a strong distinction (and corresponding associations) between *required positional* arguments and *optional* keyword-like arguments (ie, any argument with leading dashes).
In the model Rajeev has shown so far, the "scope" argument (list, domain, user) hasn't been optional.
mmclient set --key <name> --value <value>
This seems unnecessarily verbose on the one hand, and to not actually correspond to an actual use case, on the other: there's no scope mentioned. I feel the scope should be mandatory, even if it's sitewide:
mmclient set --site-wide --key CAN_PERSONALIZE --value No
mmclient set --domain=python.org --key CAN_PERSONALIZE --value Yes(after the first one, the second would be an error, I guess, but in other cases a site-wide setting would be interpreted as a default).
I guess this horse has already bolted the barn, but I wonder about a syntax like
mmclient set --site-wide --key PERSONALIZE --value Permitted
mmclient set --domain=python.org --key PERSONALIZE --value Permitted
mmclient set --list=mailman-users@python.org --key PERSONALIZE --value Nofor resource constraining settings. (Permitted could probably be an alias for False.)
Steve
On Monday 12 May 2014 10:15 PM, Stephen J. Turnbull wrote:
Barry Warsaw writes:
Also as a general rule, I think we want just one level of subcommand, so that
mmclient show --listwould be the template. (That's open to discussion.)I wonder about this in the context of argparse and the command line, because argparse makes a strong distinction (and corresponding associations) between *required positional* arguments and *optional* keyword-like arguments (ie, any argument with leading dashes).
Positional arguments *can be made* optional, also be supplied with a default value, in case the argument was not specified.
In my opinion, I don't like the one level of sub command as
neither the user nor the developer is benefited of such a design.
The user ends up typing the same strings as before plus an extra
-- followed by the same set of arguments.
And from the angle of implementation, each of the *scope* name would require a different optional argument, followed by a set of if-else's to land at the right *scope* to manage.
Further there is a possibility of the user specifying multiple scopes,
mmclient show --list --listname "list@domain.org" --domain
which makes the outcome dependent on the order in which the
if-else's are written. This is a serious bug when actions like delete
are being used.
In the model Rajeev has shown so far, the "scope" argument (list, domain, user) hasn't been optional.
I assumed this model was OK since I had received no comments against that part, since the beginning. I strongly believe that it is quite effective to mention the scope this way.
mmclient set --key <name> --value <value>
This seems unnecessarily verbose on the one hand, and to not actually correspond to an actual use case, on the other: there's no scope mentioned. I feel the scope should be mandatory, even if it's sitewide:
mmclient set --site-wide --key CAN_PERSONALIZE --value No mmclient set --domain=python.org --key CAN_PERSONALIZE --value Yes(after the first one, the second would be an error, I guess, but in other cases a site-wide setting would be interpreted as a default).
Got a bit confused with the use of *scope* in this context.
Anyways, if the scope is not specified, apply the setting on a
default *scope*, default=site-wide makes sense, while others
do not.
I guess this horse has already bolted the barn, but I wonder about a syntax like
mmclient set --site-wide --key PERSONALIZE --value Permitted mmclient set --domain=python.org --key PERSONALIZE --value Permitted mmclient set --list=mailman-users@python.org --key PERSONALIZE --value Nofor resource constraining settings. (Permitted could probably be an alias for False.)
Sorry about the horse :). As I said, I assumed it was OK, and It was a mistake from my part not to discuss the command syntax before working on it.
Also, the above is still possible with the current version. The *scope*
positional argument can
be made to default to a *scope* that has no solid structure, settings for
example. More generally, it could be defaulted to a general scope, managed
by a General class, that inherits from multiple classes like Settings,
Backup/restore etc.
And as the final word, I am ready to change the command style,
mmclient <action> <scope> <arguments>
if there is some serious disagreement with it.
Steve
On Monday 12 May 2014 09:12 PM, Barry Warsaw wrote:
A better name might be
showsince the term "list" is so overloaded in this context. Here's it's being used as a verb and a noun to refer to different concepts, and I think that's confusing.
Yes, its confusing. In fact, I was looking for a replacement for that. Thanks.
Also as a general rule, I think we want just one level of subcommand, so that
mmclient show --listwould be the template. (That's open to discussion.)...
Yuck. Sorry but I'd like to discourage the use of made up or concatenated words. It's okay for some options to be multi-word separated by dashes, e.g. --affect-bar or --change-foo.
I don't have a good alternative atm.
Will do.
E.g.
mmclient set --key <name> --value <value>
I have answered this part and the one level subcommand part in the
reply to Steve's mail.
On May 13, 2014, at 01:45 AM, Stephen J. Turnbull wrote:
In the model Rajeev has shown so far, the "scope" argument (list, domain, user) hasn't been optional.
If it's truly non-optional in the sense that there's no default, and the scope is required, then maybe it's okay. It just doesn't look "normal" to me.
mmclient set --key <name> --value <value>
This seems unnecessarily verbose on the one hand, and to not actually correspond to an actual use case, on the other: there's no scope mentioned. I feel the scope should be mandatory, even if it's sitewide:
mmclient set --site-wide --key CAN_PERSONALIZE --value No mmclient set --domain=python.org --key CAN_PERSONALIZE --value Yes
This seems different than what Rajeev wrote, where he mentions that the 'setting' argument is the scope:
On May 11, 2014, at 10:37 PM, Rajeev S wrote:
*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>
On May 13, 2014, at 01:45 AM, Stephen J. Turnbull wrote:
(after the first one, the second would be an error, I guess, but in other cases a site-wide setting would be interpreted as a default).
I guess this horse has already bolted the barn, but I wonder about a syntax like
mmclient set --site-wide --key PERSONALIZE --value Permitted mmclient set --domain=python.org --key PERSONALIZE --value Permitted mmclient set --list=mailman-users@python.org --key PERSONALIZE --value No
I think that would be fine (probably --site is sufficient, and a bit shorter). Also I would suggest allowing either List-ID or posting address as an argument to --list, e.g. --list=mailman-users.python.org would also be acceptable. Usually there will be no difference, but lists can be renamed and the List-ID will never change.
Cheers, -Barry
On May 13, 2014, at 03:27 PM, Rajeev S wrote:
Further there is a possibility of the user specifying multiple scopes,
mmclient show --list --listname "list@domain.org" --domain
Would --list be implied by seeing a --listname=list@example.com? E.g. would
this be just as useful, and a little shorter:
mmclient show --listname=list@example.com --domain=example.org
?
which makes the outcome dependent on the order in which the if-else's are written. This is a serious bug when actions like
deleteare being used.
Destructive actions should probably be more constrained in what they allow, so that there's no possibility of ambiguity on either the part of the user or the code. "In the face of ambiguity, refuse the temptation to guess."
Got a bit confused with the use of *scope* in this context. Anyways, if the scope is not specified, apply the setting on a default *scope*,
default=site-widemakes sense, while others do not.
Hmm. If scope is optional (because it has a default), then it's not a required positional argument, right? So shouldn't a --switch should be used?
Sorry about the horse :). As I said, I assumed it was OK, and It was a mistake from my part not to discuss the command syntax before working on it.
Also, the above is still possible with the current version. The *scope* positional argument can be made to default to a *scope* that has no solid structure,
settingsfor example. More generally, it could be defaulted to ageneralscope, managed by aGeneralclass, that inherits from multiple classes likeSettings,Backup/restoreetc.And as the final word, I am ready to change the command style,
mmclient <action> <scope> <arguments>
if there is some serious disagreement with it.
I just want it to be consistent, easily described, and easily understood by users. If it makes sense for the mmclient CLI to different from the shell-access mailman command, then we at least need to be "translatable" between the two.
-Barry
BTW, unless specifically mentioned that I'm speaking as mentor, I'm speaking as an ordinary developer, and you should feel free to argue with me, or agree with me, or reserve comment until you feel comfortable discussing issues. Also, I apologize if I end up talking "down" to you. I don't know you very well yet, so feel free to let me know you already understand what I'm talking about.
Rajeev S writes:
Positional arguments *can be made* optional, also be supplied with a default value, in case the argument was not specified.
Sure, I'm just saying that argparse "likes" to associate "positional" with "required", and keyword-like with "optional". We should consider carefully whether we want to deviate from that practice because there's probably good reason for it.
In my opinion, I don't like the
one level of sub command
That's fine by me, and as your mentor I advise that you continue to discuss this with Barry. I hope that you and he will come to agreement on this point. In the end, however, Barry is The Client, and if you don't come to a meeting of minds the default is to do it his way. OK?
Taking my mentor hat off, now.
Further there is a possibility of the user specifying multiple scopes,
mmclient show --list --listname "list@domain.org" --domain
I think this is a syntax error, and should be reported that way.
In the model Rajeev has shown so far, the "scope" argument (list, domain, user) hasn't been optional.
I assumed this model was OK since I had received no comments against that part, since the beginning. I strongly believe that it is quite effective to mention the scope this way.
I agree with that. Note, however, that it has been suggested that in the shell it should be possible to
set scope list=foo-list@domain.org
so that after that only list-scope commands are allowed and only foo-list will be affected.
Sorry about the horse :). As I said, I assumed it was OK, and It was a mistake from my part not to discuss the command syntax before working on it.
Again speaking as mentor, I would not call it a "mistake" in your case, but rather say it is up to your personal preference. You have demonstrated that you do write code that can be refactored and that you refactor spontaneously. It's acceptable for developers to "write code for discussion" to the extent that they are comfortable with such refactoring. Many clients will find that a great convenience (it's a simple form of "prototype").
Conversely it's unacceptable for you to respond to The Client's requirements by saying "I've already written the code and it doesn't do that." (Except, of course, if the existing code was written to a specification accepted by the client and he's changing his mind. Then both sides have a responsibility to negotiate.)
Also, the above is still possible with the current version. The *scope* positional argument can be made to default to a *scope* that has no solid structure,
settingsfor example. More generally, it could be defaulted to ageneralscope, managed by aGeneralclass, that inherits from multiple classes likeSettings,Backup/restoreetc.
Wearing my developer hat, I don't much like an implicit default scope. The user should either specify the scope in each command, or explicitly specify a default scope.
Regards, Steve
Barry Warsaw writes:
Would --list be implied by seeing a
--listname=list@example.com? E.g. would this be just as useful, and a little shorter:mmclient show --listname=list@example.com --domain=example.org
Are you thinking that this is equivalent to
mmclient show --list --listname=list@example.com
mmclient show --domain=example.orgwhich would display the set of subscribers for list@example.com and the set of lists for example.org? I can see that as a minor convenience, but it doesn't seem useful enough to allow.
"In the face of ambiguity, refuse the temptation to guess."
Rajeev, do you know about the Zen of Python? If not yet, try "python -m this". :-)
I just want it to be consistent, easily described, and easily understood by users. If it makes sense for the mmclient CLI to different from the shell-access mailman command, then we at least need to be "translatable" between the two.
What do you mean by "shell-access mailman command"? src/mailman/bin/mailman.py?
On Tuesday 13 May 2014 07:12 PM, Barry Warsaw wrote:
On May 13, 2014, at 03:27 PM, Rajeev S wrote:
Would --list be implied by seeing a
--listname=list@example.com? E.g. would this be just as useful, and a little shorter:mmclient show --listname=list@example.com --domain=example.org
?
In the command *mmclient <scope> <action>* , the scope denotes the class name and the action denotes the class method to be invoked.
The following format suggested by you,
mmclient show --listname=list@example.com --domain=example.org
From the developer point of view, its difficult to map an action to a class by using this format. For eg, the above command can be associated with the class domain or list, as the order of the arguments is insignificant.
Further, the commands do not offer much advantage to the user in terms of usability. Most of the current (frequent) commands are quite straightforward and similar to the ones to be used in the shell, for eg, *create list list@domain.org* compared to *create --listname="list@domain.org"*.
Some commands like *add-role* are less intuitive in this format,yet provide a tolerable level of understandability.
which makes the outcome dependent on the order in which the if-else's are written. This is a serious bug when actions like
deleteare being used. Destructive actions should probably be more constrained in what they allow, so that there's no possibility of ambiguity on either the part of the user or the code. "In the face of ambiguity, refuse the temptation to guess."Got a bit confused with the use of *scope* in this context. Anyways, if the scope is not specified, apply the setting on a default *scope*,
default=site-widemakes sense, while others do not. Hmm. If scope is optional (because it has a default), then it's not a required positional argument, right? So shouldn't a --switch should be used?
A switch would work best when the number of possible choices are very few. Here we have the following choices for the scope, site-wide, domain and list. It seems OK to use a switch here.
However, using switches limits the extendibility of the scopes. If a new scope is to be added some day (something server-wide?), integrating new switches is not an easy task, when compared to the addition of a new choice for scope in current approach.
And as the final word, I am ready to change the command style,
mmclient <action> <scope> <arguments>
if there is some serious disagreement with it. I just want it to be consistent, easily described, and easily understood by users. If it makes sense for the mmclient CLI to different from the shell-access mailman command, then we at least need to be "translatable" between the two.
The current format of commands,mmclient <scope> <action> <arguments> is
directly translatable to the shell. In fact, they are almost similar,
except for the --
in the commands.
On May 14, 2014, at 12:19 PM, Stephen J. Turnbull wrote:
Barry Warsaw writes:
I just want it to be consistent, easily described, and easily understood by users. If it makes sense for the mmclient CLI to different from the shell-access mailman command, then we at least need to be "translatable" between the two.
What do you mean by "shell-access mailman command"? src/mailman/bin/mailman.py?
Yes, as much as makes sense. When installed into a virtual environment,
you'll have <venv>/bin/mailman with lots of subcommands, many of which are
similar to mmclient. It may not be possible to give them identical
signatures, and that's fine, but it would be great if when someone learns the
commands for one, the other will not break their intuition, especially for
things that are "close".
Cheers, -Barry
participants (3)
-
Barry Warsaw -
Rajeev S -
Stephen J. Turnbull