Re: mailgw argument parsing

Hi Ralf:

In message <[email protected]>,
Ralf Schlatterbeck writes:
>I'm in the process of adding OAUTH support to roundup-mailgw.

Neat. Do you know about: https://issues.roundup-tracker.org/issue2551239?

>Now the current argument parsing in mailgw uses getopt.getopt for
>argument parsing. I'd like to move this to argparse.ArgumentParser when
>at it (I need to add at least one new option).

I have no problem with that. I think argparse worked pretty much the
same in Python2.

We have an open ticket on replacing optparse with argparse:

     https://issues.roundup-tracker.org/issue2551195

standardizing on argparse would be good. Drop any hints on using
argparse in the ticket.

>There is a -c option to set the Class we're acting on, defaulting to
>MAIL_DEFAULT_CLASS (in the config-file default_class in section [mailgw])
>
>The -C and -S options to mailgw are used to set properties on a class,
>-C accepts the values 'file', 'issue', 'msg', 'user' and defaults to 'msg'.
>Note that even if a -c option is present, setting the default class to
>something other than 'issue' the -C option will insist on 'issue'. At
>least that's how I understand the code.

Hmm... that seems wrong. -C is usually msg., but I have never used
'-c' so.... that's possible.

My use case was to categorize messages into "reply" and "comment"
types arriving on different email addresses.  The two addresses
notified different nosy lists. So:

   -C msg -S "messagetype=reply" (notifies nosy and verynosy list)
   -C msg -S "messagetype=comment"  (notifies only on the verynosy list)

I was using the issue class in both cases.

I'm not actually sure when you would use -c to change the class to
"file", "msg" etc. I could see -c "bugs" -c "enhancement" for
different classes of issues. But I am not sure how using "user" would
work.  I think the mailgw requires something with the shape of an
issue with messages and attachments/files??

>So to set several properties on e.g. issue and user you would use
>
>-C issue -S issueprop1=value1 -S issueprop2=value2 -C user -S userprop=uvalue
>
>This interleaving of -C and -S options is not only un-intuitive for
>users (well, at least for me)

Agreed, not intuitive but the man page only allows one pair of -C/-S
where -C is optional:

  ... [[-C  class] -S field=value] ...

If it parses multiple pairs, I am going to claim that is a bug.

I suppose you could do something like

  -C file -S visibility=hidden

to make all  attached files via that email address hidden or

  -c msg -S messagetype=reply -C issue -S visibility=hidden

when you tell people to send security issues to

  [email protected]

to make the issue invisible when created.

>but is fundamentally incompatible with how ArgumentParser works.

I have only taken a glance at argparse (when I opened the issue) and
thought multiple flags could be parsed into a list using append. But
in this case supporting multiple pairs of -C/-S would require a -C
before the second -S. Then you just zip the lists together. But this
does make -C manadatory.

>So I'd suggest we change this to only a -S (--set-value) option that
>optionally takes a classname before a '.', the above would become:
>
>-S issue.issueprop1=value1 -S issue.issueprop2=value2 -S user.userprop=uvalue

How about still supporting -C msg so:

 -C file -S fileprop1=value1 -S issue.issueprop2=value2 -S user.userprop=uvalue

provides the default if no class is specified.

So in this case it would be the same as if the first -S argument was:

   file.fileprop1=value1

and the default is still msg if -C isn't used.

The advantage is that existing invocations don't change and you get to
set multiple things from the email address.

Thoughts?

Have a great week.

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.