Re: mailgw argument parsing

Hi Ralf:

In message <[email protected]>,
Ralf Schlatterbeck writes:
>On Wed, Nov 23, 2022 at 10:58:02AM -0500, John P. Rouillard wrote:
>> >My current implementation ignores these calls when the classname does
>> >not match.
>> 
>> Do you think it should log instead? Something like:
>> 
>>    ignoring -S option foo.private=true as message is for class keyword
>
>No, a better wording would have been that the class is now matched
>exactly.

Sorry, I don't understand which class? The one specified by -c (issue
by default) or the one in the -S?

>So you no longer specify -C issue but use the real class name
>which is your issue class (e.g. -S support:prop=value).

But that's not all -C did. The -C and -S did what couldn't be done
from the option/attribute setting on the subject line. They changed
the properties not of the issue but of the msg created by sending the
email to the issue.

It would be the equivalent subject setting mechanism as if we had
implemented:

   [msg-1:msgtype=reply]

in the subject command line, if that makes sense.

In theory you could have created a timelog entry (how, TBD) from the
email.  So emailing:

  billable: | mailgw -C timelog -S billable=True
  unbillable: | mailgw -C timelog -S billable=False

I recall some discussion about how to implemement timelog via email,
around the time Richard added it to customizing.txt, but I can't find
it.

>And you now can have several different issue-like classes and specify
>(different) -S option for all of them.

Sorry, I'm not getting it. How would I rewrite my existing
roundup-mailgw alias from:

  roundup-mailgw -c issue -C msg -S msgtype=reply

that:

  takes an incoming email
  creates a new message with a msgtype of reply
  adds the id of the new msg to the messages propery of the issue

Would

  roundup-mailgw -c issue -S msgtype=reply

be correct? If somebody sent in an email with the subject

    Subject: [issueA2] some subject

the -S option would be ignored since issueA (an alternate issue class)
doesn't match what is specified by "-c issue"?

While

    Subject: [issue2] some other subject

will set the msgtype on the msg created by the email to the issue as
expected?

What happens if I use:

  roundup-mailgw -c issue -S msg.msgtype=reply

is it the same as with -S msgtype-reply?

>> Even though -S is used rarely, I can see debugging this being an issue.
>> Also what happens if you have to issue classes:
>> 
>>   bug and feature
>> 
>> with the same set of properties. If a subject [bug12345] comes in and
>> you have:
>> 
>>   -S feature.status=low
>> 
>> it should be ignored. Right??
>
>Yes. It is, see above: You now specify
>
>-S bug.bugprop=v1 -S feature.feaprop=v2

This was a poor example as the use case for -C and -S was to apply to
newly created object. But I understand. That's reasonable.

>you no longer specify issue-type but the real class name.
>And you can even have -S keyword.prop=v and when someone composes a
>message with Subject: [keyword1] it will be applied :-)

Hmm, I am not sure that's a good thing. I need to think about it.  I
claim that subject should result in a bounce since a keyword is not an
issue like thing.

>>   -S status=low
>> 
>> what happens? Applied??
>
>Applied to the default 'msg'. If the classname is left out of the -S
>option it defaults to msg.

Understood. So the only way to change a property on the msg is to not
set a classname. Also it will apply to whatever issue type is
specified with -c (issue by default)?

What happens if the designator is [keyword23]? Keywords don't have
messages so it's ignored?

>> This could lead to a lot of logging. Maybe a flag to enable debug
>> logging and put these "ignoring -S option foo...." options behind it?
>> Thoughts?
>
>See above: For each message only a subset of all possible -S options are
>used, I guess the debug logging would confuse more than help.

Well the intent was only to enable it while debugging. In production
it would be off.

>I'm also
>using a dict for the various classes where I put all -S options for that
>class, so it would take an additional iteration to log the unused
>classes.

Understood. But reasonable when hidden behind a debug flag.

>And if what you're doing with the -S option becomes really complicated,
>using an auditor instead should be considered.

I'm not sure you can do that. IIUC the inbound email address is gone
by the time the auditor gets involved. You could add a
"received_address" property to the message and use -S to set it. Then
that might be usable from an auditor somehow?? You wouldn't want to
add it to the issue as that would just churn that field on every
email.

>> Also it looks like the 'Mail gateway script command line' is
>> documented in user-guide.txt. Any idea why? Maybe after your changes
>> we can move the revised command line/setup info into admin-guide. This
>> still leaves more technical mailgw stuff in the user guide but is a
>> start to cleaning it up.
>
>Probably historical reasons, I agree it should be moved to the admin
>guide.

Good.

>Maybe we can even come up with a way to re-use the mailgw help
>(it's now implemented as the epilog argument of ArgumentParser):

I saw.

>This is
>(almost) the same text as in the current section in the user guide. It
>had drifted apart considerably and one of the things I did was to
>consolidate the two. But it should really be a common source.

Agreed. The man page, admin guide and internal help should all be
generated off the same source. Let me know if you have any bright
ideas 8-).

>ArgumentParser has options for how the epilog is formatted,
>unfortunately none for formatting ReStructuredText...

Yeah I ended up using formatter_class=argparse.RawTextHelpFormatter in
my rewrite of arg parsing for roundup_demo as well.

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