Re: [Mailman-Developers] [GSoC 2014] Mailman CLI Project
Hi,
I have pushed the revision 53, that has the following changes.
- Refactoring as mentioned in the comments by Steve
- Grouping of options and sub commands using subparsers
- Changed the format of the command from *mmclient <scope> <action>* to *mmclient <action> <scope>*
- Replaced ambiguous action *list* with *show*
The change mentioned in point 2 solves many/most of the problems raised during the recent discussions.
The options are now paired with their corresponding sub commands. The implications of this change is that the extra options/arguments raise command syntax errors, which were ignored earlier. Further, an great extent of leniency can be allowed in the command format. To be specific, the parser can support the commands
*mmclient create list --list list1 --domain domain.org*
and
*mmclient set --key "foo" --value "bar"*
at the same time. Note that the second command has not specified a scope.(PS: scope as in list, domain, user etc, and not site-wide, domain etc).
The change also prevents argparse from generating a dictionary with all possible options (which may be huge).
The agreed change to add a positional argument for the *name* (commands of the form *mmclient create list list@domain.org*) has been postponed to the next revision, as this revision mostly contains code refactoring.
http://bazaar.launchpad.net/~rajeevs1992/mailman.client/mailmancli/revision/...
As discussed, I have completed the r54, that does the following
- The arguments will be specified as positional arguments wherever necessary and possible. The ideal way to use positional arguments here is "if the argument to the command is the name of an instance the scope, use positional args else use optional args"
E.g, To filter mailing lists based on domains, do
mmclient show list --domain domain.org
and not
mmclient show list domain.org
To create a list, do
mmclient create list list@domain.org
instead of
mmclient create list --list list --domain domain.org
- Added a no-header option to the detailed listing so that it becomes easier to pip the output.
I will be making the following changes for r55
1.Delete
Quite a straightforward one.
delete list list@domain.org delete domain domain.org
Begin the third
scopeuser and add methods like create, delete and list.create user foo@bar.com --password PASS --name NAME list user [--verbose] [--no-header]
The user class would be built and managed in the same way as other classes.
r55 would be an easy one. Once the above changes are approved, r55 can be completed in a day.
Rajeev S <rajeevs1992@gmail.com> wrote:
As discussed, I have completed the r54, that does the following
- The arguments will be specified as positional arguments wherever necessary and possible. The ideal way to use positional arguments here is "if the argument to the command is the name of an instance the scope, use positional args else use optional args"
E.g, To filter mailing lists based on domains, do
mmclient show list --domain domain.org
and not
mmclient show list domain.org
To create a list, do
mmclient create list list@domain.org
instead of
mmclient create list --list list --domain domain.org
Both of these options looks clean to me as a user. So if I am right you have completed
- Listing of domains
- Listing of mailing lists(filtered by domain also)
- Creating domains
- Creating mailing lists
Right?
Since this tool is meant for the users, you should write better
documentation. Like in using.txt#39 what does long listing mean? What
does --verbose do for listing a domain? Or even for listing all the
lists?
As I understand using.txt is more of a command reference than documentation?
Are there only these two options for lists and domains? What about
editing any list? Adding and removing user roles will be possible after
you have create the user scope but editing of other parameters can be
done?
Also maybe you can try making your tool a little more smart? Like lets say I try to create a list abhilash@raj.com and there is no domain raj.com in the database, so instead of just showing error maybe you can ask the user:
"The domain raj.com does not exist, Do you want to create one? [y/n]"
Just adding all the options using argparse is really not a very tough job (and with your pace, it is definitely not going to last more than one month ;-). Try to put some more thought to how you can make this tool more intuitive for the end user.
- Added a no-header option to the detailed listing so that it becomes easier to pip the output.
I will be making the following changes for r55
1.Delete
Quite a straightforward one.
delete list list@domain.org delete domain domain.org
What I belive is that deleting a domain should not strightforward, any destructive command should not be. Would it not be nice to prompt the user once before deleting? as in
"There are 100 lists associated with this domain. Once deleted you cannot undo it, Do you really want to delete xxx@yyy.com?"[y/n]
Or maybe it could schedule a deletion after a pre-defined time with a reasonable default lets say "1 Day"? And for an urgency(to delete) there could be --force argument?
Begin the third
scopeuser and add methods like create, delete and list.create user foo@bar.com --password PASS --name NAME list user [--verbose] [--no-header]
The user class would be built and managed in the same way as other classes.
r55 would be an easy one. Once the above changes are approved, r55 can be completed in a day.
I roughly went through your code, and I have a few more points:
- You should write tests, before writing more code. Infact you should follow TDD (ofcourse if you are comfortable doing it) since the outcome of your commands is less likely to change, even though your code does.
- The commit messages should be written in a language that tells a reader "What this commit does?". Like "Adds documentation file using.txt". Instad of answering "What I have done in this commit?" Although it is my point of view, AFAIK it is also preferred by many other developers.(See commit messages of barry on lp:mailman)
Mailman-Developers mailing list Mailman-Developers@python.org https://mail.python.org/mailman/listinfo/mailman-developers Mailman FAQ: http://wiki.list.org/x/AgA3 Searchable Archives: http://www.mail-archive.com/mailman-developers%40python.org/ Unsubscribe: https://mail.python.org/mailman/options/mailman-developers/raj.abhilash1%40g...
Security Policy: http://wiki.list.org/x/QIA9
-- thanks, Abhilash Raj
On Tuesday 27 May 2014 12:27 PM, Abhilash Raj wrote:
Rajeev S <rajeevs1992@gmail.com> wrote:
Both of these options looks clean to me as a user. So if I am right you have completed
- Listing of domains
- Listing of mailing lists(filtered by domain also)
- Creating domains
- Creating mailing lists
Right?
Since this tool is meant for the users, you should write better documentation. Like in using.txt#39 what does long listing mean? What does
--verbosedo for listing a domain? Or even for listing all the lists?As I understand using.txt is more of a command reference than documentation?
Are there only these two options for lists and domains? What about editing any list? Adding and removing user roles will be possible after you have create the
userscope but editing of other parameters can be done?
No, there are more options for lists and domains, many of which are dependent on the users. Some of the other options are list-members, edit and export(to CSV)/import. Hence, these would be easier to build after a basic user class is available.
Also maybe you can try making your tool a little more smart? Like lets say I try to create a list abhilash@raj.com and there is no domain raj.com in the database, so instead of just showing error maybe you can ask the user:
"The domain raj.com does not exist, Do you want to create one? [y/n]"
Just adding all the options using argparse is really not a very tough job (and with your pace, it is definitely not going to last more than one month ;-). Try to put some more thought to how you can make this tool more intuitive for the end user. What I belive is that deleting a domain should not strightforward, any destructive command should not be. Would it not be nice to prompt the user once before deleting? as in
"There are 100 lists associated with this domain. Once deleted you cannot undo it, Do you really want to delete xxx@yyy.com?"[y/n]
Or maybe it could schedule a deletion after a pre-defined time with a reasonable default lets say "1 Day"? And for an urgency(to delete) there could be --force argument?
I roughly went through your code, and I have a few more points:
- You should write tests, before writing more code. Infact you should follow TDD (ofcourse if you are comfortable doing it) since the outcome of your commands is less likely to change, even though your code does.
- The commit messages should be written in a language that tells a reader "What this commit does?". Like "Adds documentation file using.txt". Instad of answering "What I have done in this commit?" Although it is my point of view, AFAIK it is also preferred by many other developers.(See commit messages of barry on lp:mailman)
Everything else seems fine. Will apply the necessary changes.
Rajeev S writes:
Also maybe you can try making your tool a little more smart? Like lets say I try to create a list abhilash@raj.com and there is no domain raj.com in the database, so instead of just showing error maybe you can ask the user:
"The domain raj.com does not exist, Do you want to create one? [y/n]"
I'm not sure I like this approach. Creating a domain should be a heavyweight operation, and eventually should include a bunch of sanity checks, like existence of domains and MX records. I have visions (nightmares) of users coming to us saying
User: I said yes, but mail never arrives.
Dev: .... Oh, is there a proper entry in the DNS?
User: Doesn't Mailman create the domain?> > Or maybe it could schedule a deletion after a pre-defined time with a
reasonable default lets say "1 Day"? And for an urgency(to delete) there could be --force argument?
Deleting a list should be immediate, but I agree it should be confirmed.
- Stephen J. Turnbull <stephen@xemacs.org>:
Rajeev S writes:
Also maybe you can try making your tool a little more smart? Like lets say I try to create a list abhilash@raj.com and there is no domain raj.com in the database, so instead of just showing error maybe you can ask the user:
"The domain raj.com does not exist, Do you want to create one? [y/n]"
I'm not sure I like this approach. Creating a domain should be a heavyweight operation, and eventually should include a bunch of sanity checks, like existence of domains and MX records. I have visions (nightmares) of users coming to us saying
User: I said yes, but mail never arrives. Dev: .... Oh, is there a proper entry in the DNS? User: Doesn't Mailman create the domain?
I doubt anyone that igorant of e-mail and how it works will ever make it to the MM3 command line client, but yes, such cases do exist. We should have public pillory that receives name, mail and date, whenever someone confirms the above question. ;)
However I think the use case "prepare Mailman to handle mail for a domain it doesn't handle mail for yet" exists and we should find a way to deal with it.
Perhaps we could improve this, if we used better wording that doesn't lead the operator to believe Mailman will connect a domain, setup DNS, do all the other foo and finally configure it self to handle mailing lists for that domain?
Or maybe it could schedule a deletion after a pre-defined time with a reasonable default lets say "1 Day"? And for an urgency(to delete) there
Deleting a list/domain requires an (internal) scheduler. Does Mailman have one? A broom job that can be called via cron?
could be --force argument?
Deleting a list should be immediate, but I agree it should be confirmed.
... and it should be possible to pass the confirmation in the command to make it useful in scripts.
p@rick
-- [*] sys4 AG
https://sys4.de, +49 (89) 30 90 46 64 Franziskanerstraße 15, 81669 München
Sitz der Gesellschaft: München, Amtsgericht München: HRB 199263 Vorstand: Patrick Ben Koetter, Marc Schiffbauer Aufsichtsratsvorsitzender: Florian Kirstein
On May 29, 2014, at 05:20 PM, Patrick Ben Koetter wrote:
Deleting a list/domain requires an (internal) scheduler. Does Mailman have one? A broom job that can be called via cron?
Sort of, but the way these are handled currently are individual scripts that each have to be added to cron.
... and it should be possible to pass the confirmation in the command to make it useful in scripts.
Many such scripts have a --yes option to pre-confirm the action.
-Barry
Patrick Ben Koetter writes:
I doubt anyone that igorant of e-mail and how it works will ever make it to the MM3 command line client, but yes, such cases do exist.
I think they're actually likely to be reasonably common.
However I think the use case "prepare Mailman to handle mail for a domain it doesn't handle mail for yet" exists and we should find a way to deal with it.
Indeed. I'm suggesting that the routine could check the DNS and a few other things, and present the list operator with a TODO list. Sortof like check_perms.
Deleting a list should be immediate, but I agree it should be confirmed.
... and it should be possible to pass the confirmation in the command to make it useful in scripts.
Ooh, that is a very good point. Bessides "possible," the syntax for doing that should be regular.
I wonder about distinguishing "yes to this prompt" and "yes to all"? Probably scripts should not be using commands that need multiple confirmations, though.
- Stephen J. Turnbull <stephen@xemacs.org>:
Patrick Ben Koetter writes:
I doubt anyone that igorant of e-mail and how it works will ever make it to the MM3 command line client, but yes, such cases do exist.
I think they're actually likely to be reasonably common.
However I think the use case "prepare Mailman to handle mail for a domain it doesn't handle mail for yet" exists and we should find a way to deal with it.
Indeed. I'm suggesting that the routine could check the DNS and a few other things, and present the list operator with a TODO list. Sortof like check_perms.
Where would you start with the TODO list? Where would you end?
Deleting a list should be immediate, but I agree it should be confirmed.
... and it should be possible to pass the confirmation in the command to make it useful in scripts.
Ooh, that is a very good point. Bessides "possible," the syntax for doing that should be regular.
I'm hopping in on this and I certainly missed most of the discussion on the command line client: Do we have interface guidelines for commands? Shortopts, longopts, interaction, non-interactive behaviour/requirements?
I wonder about distinguishing "yes to this prompt" and "yes to all"?
Me too.
Probably scripts should not be using commands that need multiple confirmations, though.
Hmmm, Unix: Do one thing. Do it well. ?
-- [*] sys4 AG
https://sys4.de, +49 (89) 30 90 46 64 Franziskanerstraße 15, 81669 München
Sitz der Gesellschaft: München, Amtsgericht München: HRB 199263 Vorstand: Patrick Ben Koetter, Marc Schiffbauer Aufsichtsratsvorsitzender: Florian Kirstein
Hi,
I have made more additions to the CLI, like the delete command and the
user scope.
Following is the list of changes brought about
1.Introduce User scope
Actions supported by User scope currently are create, show and delete. show users, like other scopes, supports verbose and no-header flags. However the detailed listing of users is not quite effective as most of the fields are lists rather than single valued attributes. The non detailed version prints one of the email ids of each user. The show list command takes an optional argument list name that gives the users who are members of the list.
2.Introduce a cli/lib directory, to store procedures common across the application.
The directory is to hold the procedures and classes that are reused among scripts. It currently holds a class that is used to colorize the output printed on the terminal.
3.Updated and improved docs/using.txt. It was obsolete in r54.
4.Added delete action for all objects
The action is confirmed by a question to user, which can be overridden by a --yes switch.
I will be committing and pushing r55 tonight (30/05/2014 around 16:30 UST, I am away from the computer right now).
As mentioned by Steve and agreed by Barry, I too believe that, in the scenario of the CLI multiple confirmations won't be necessary. In a situation in which such multiple confirmations come necessary, I believe that the commands will be better if the confirmations are replaced by switches.
I agree that an override for the confirmation message is necessary so that
the CLI
commands can be used in scripts and I would like to follow Barry's
suggestion of using
a --yes or --force switch. (Isn't --force more common than --yes, as in
rm -f ?)
About the create domain suggestion, Is the follow up question on *create domain*, upon missing domain,
such a bad idea? A user already can add a domain by using the web
interface. If that is not ambiguous,
neither is this, right?
However I see this issue, if a newbie user types in an email address in place of the list name, probably on misreading the command format, he would end up creating a domain for his mail provider.
*$./mmclient create list --help* *Creates the list list@domain.org <list@domain.org>* *$./mmclient create list user@gmail.com <user@gmail.com>* *The domain gmail.com <http://gmail.com> does not exist.Create one?[y/n] y* *$*
@Abhilash
I have written unittests for methods like connect and create, but have to refine them more. Hence the tests won't come up in this revision.
Also, I would like to ask how the tests should be triggered. Should
there be a mmclient test with a set of possible switches like all
,domain, list, user etc?
Or should the customary python setup.py test be used?
I also propose to build a a *describe user* command that gives a detailed
profile of the user.
Verbose listing using tables is not so effective users as most of the
attributes are lists.
Thanks!
On Thu, 29 May 2014 22:30:25 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote:
Also maybe you can try making your tool a little more smart? Like lets say I try to create a list abhilash@raj.com and there is no domain raj.com in the database, so instead of just showing error maybe you can ask the user:
"The domain raj.com does not exist, Do you want to create one? [y/n]"
I'm not sure I like this approach. Creating a domain should be a heavyweight operation, and eventually should include a bunch of sanity checks, like existence of domains and MX records. I have visions (nightmares) of users coming to us saying
User: I said yes, but mail never arrives. Dev: .... Oh, is there a proper entry in the DNS? User: Doesn't Mailman create the domain?
I agree to your point that it should be a heavyweight operation, I guess i was not able to express myself. I don't know if this CLI client can add domains to mailman(I mean add domain to mailman before and not *create*), there could be a prompt saying "This mailman installation is not configured to use this domain, do you want to do it now?" and then maybe it will walk the user through the "usual" process of adding a new domain?
Or maybe it could schedule a deletion after a pre-defined time with a reasonable default lets say "1 Day"? And for an urgency(to delete) there could be --force argument?
Deleting a list should be immediate, but I agree it should be confirmed.
-- thanks, Abhilash Raj
Although I haven't had time to go through the code, I'm liking what I'm seeing here on the mailing list. Just a quick comment.
On May 27, 2014, at 12:27 PM, Abhilash Raj wrote:
Since this tool is meant for the users, you should write better documentation. Like in using.txt
Ideally, because this is a command line tool aimed and users, there should be a manpage for mmclient. Fortunately, it's *really* easy to write manpages in reST with Sphinx.
http://sphinx-doc.org/builders.html#sphinx.builders.manpage.ManualPageBuilde...
And since examples never hurt:
Cheers, -Barry
Hi Barry,
Although I haven't had time to go through the code, I'm liking what I'm
seeing here on the mailing list. Just a quick comment. [...] Ideally, because this is a command line tool aimed and users, there should be a manpage for mmclient. Fortunately, it's *really* easy to write manpages in reST with Sphinx.
Sure it is necessary. And writing the man page is mentioned in my proposal as a project deliverable.
http://sphinx-doc.org/builders.html#sphinx.builders.manpage.ManualPageBuilde...
And since examples never hurt:
Thanks for the links.
participants (5)
-
Abhilash Raj -
Barry Warsaw -
Patrick Ben Koetter -
Rajeev S -
Stephen J. Turnbull