Subject: Re: [sieve] I-D Action: draft-ietf-sieve-convert-02.txt

From: Alexey Melnikov <[email-masked]>
Date: Thu, 04 Aug 2011 11:09:27 -0400

Hi Stephan,

Stephan Bosch wrote:
> On 7/25/2011 8:18 PM, [email-masked] wrote:
>> A New Internet-Draft is available from the on-line Internet-Drafts 
>> directories. This draft is a work item of the Sieve Mail Filtering 
>> Language Working Group of the IETF.
>>
>>     Title           : Sieve Extension for converting messages before 
>> delivery
>>     Author(s)       : Alexey Melnikov
>>                            Qian Sun
>>                            Barry Leiba
>>                            Kepeng Li
>>     Filename        : draft-ietf-sieve-convert-02.txt
>>     Pages           : 8
>>     Date            : 2011-07-25
>>
>>     This document describes how IMAP CONVERT can be used within Sieve to
>>     transform messages before final delivery.
>>
>> A URL for this Internet-Draft is:
>> http://www.ietf.org/internet-drafts/draft-ietf-sieve-convert-02.txt
> I gave this a quick first review. I have the following questions:
>
> 1)  Do we need a mechanism to discover which conversions are available 
> (analogous to the IMAP CONVERSIONS command and the 
> AVAILABLECONVERSIONS data  item)?
We can have a new test or magic variables (in a special namespace) for that.

Do you really think that the extra complexity worth it? Maybe you have a 
use case in mind.
> 2) Error handling: what happens when the convert command is issued 
> with invalid parameters, e.g. unknown conversion, unknown/invalid  
> parameter etc. ? And what happens when a conversion just fails 
> temporarily or due to invalid input data from the message?
I think this should be a runtime error.
> 3) The interaction with subsequent disposition actions and other 
> convert actions is described explicitly. But what happens with 
> subsequent tests that evaluate the message body (e.g. header :mime) ? 
> E.g. for the editheader extension, subsequent header tests all see the 
> changes made. If tests following the convert command should see the  
> changes as well, applying the instructed conversions at the end of 
> Script execution (as suggested at the end of Section 2) will be 
> difficult.
Hmm. I was mostly thinking about leaf body parts conversions, but other 
conversions should be possible.

I agree with applying changed "on the spot" (just like editheader).

---

From: Stephan Bosch <[email-masked]>
Date: Tue, 09 Aug 2011 03:17:25 +0200

On 8/4/2011 5:09 PM, Alexey Melnikov wrote:
>> 1)  Do we need a mechanism to discover which conversions are 
>> available (analogous to the IMAP CONVERSIONS command and the 
> Do you really think that the extra complexity worth it? Maybe you have 
> a use case in mind.

Well, I think it would be good to be able avoid unnecessary runtime 
errors (as proposed below).  I guess the same considerations applied for 
the enotify extension and its valid_notify_method and 
notify_method_capability tests.

I don't have a particular use-case in mind.. yet. I won't be 
implementing this soon, so it will be hard to foresee on my part.
> I think this should be a runtime error.

Yes.

>> E.g. for the editheader extension, subsequent header tests all see 
>> the changes made. If tests following the convert command should see 
>> the  changes as well, applying the instructed conversions at the end 
>> of Script execution (as suggested at the end of Section 2) will be 
>> difficult.
> Hmm. I was mostly thinking about leaf body parts conversions, but 
> other conversions should be possible.
>
Ok.

Regards,

Stephan.

---

From: Barry Leiba <[email-masked]>
Date: Fri, 12 Aug 2011 10:14:03 -0400

First, as I said on the mic in Québec, thanks, Stephan, for great
reviews of convert and imap-sieve.  I'll be getting to the latter
soon.

Stephan has brought up two main issues (the third is easy, and I've
clarified that for the upcoming version):

1. Do we need a test for what conversions are supported?

2. How do we handle error conditions?

I've tentatively made the following change to section 2 for the second item:

OLD
   Implementations ought to defer any actual conversion until the final
   resolution of other actions, to avoid doing conversions unnecessarily
   in cases where the message is not retained (such as where the
   resolution is "discard").

NEW
   If a "convert" action cannot be completed -- perhaps because the
   conversion failed, or because the requested conversion is not
   available -- the message MUST remain unchanged, and the script
   processing continues.  In particular, no error condition is raised,
   and no partial conversions are allowed.

   Implementations might defer any actual conversion until the the
   results of the conversion are needed for script processing, to avoid
   doing conversions unnecessarily.  Consider the case wherein a
   "convert" action is processed, but a "discard" action results without
   the need to actually perform the conversion.

Now, this doesn't take into account Alexey's comment that he'd like to
see a runtime error when conversion fails, so I want to push on this:
What is the use case?  Is it more valuable to the use case for a
failed conversion to crash the script, or for it to just continue?
The sense I have is that it should continue (which is why I wrote the
text above).

If we go with my version, there's an issue of what happens when we're
converting the whole message ("convert" outside a MIME loop) and, say,
three body parts convert OK and one doesn't.  Do we consider that a
"partial conversion", and roll back the conversion on the three
successful ones?  Or do we only worry about making sure the part that
failed is back to its previous state?

Finally, I had an idea for how we could do the testing, which might
satisfy both issues:
Why not have the "convert" action *also* be a test.  Like this:

*** convert with no test ***
       require ["mime", "fileinto", "convert"];
       if header :mime :anychild :contenttype
                 "Content-Type" "image/tiff"
       {
         convert "image/tiff" "image/jpeg" "pix-x" "320" "pix-y" "240";
         fileinto "INBOX.pics";
       }

*** convert with test ***
       require ["mime", "fileinto", "convert"];
       if header :mime :anychild :contenttype
                 "Content-Type" "image/tiff"
       {
         if convert "image/tiff" "image/jpeg" "pix-x" "320" "pix-y" "240"
         {
           fileinto "INBOX.pics";
         }
       }

So it could work either way.  In the case where it's used in a test,
it would evaluate TRUE if it ran with no errors -- that is, the
conversion is supported, the parameters are correct, and no
conversions failed (Q: is it also TRUE if no conversions needed to be
done, or is it only TRUE if at least one conversion was done?).  If
it's used only as an action, with no test, then any failure generates
a runtime error.

What do people think about that?  This would be the first action that
could also be used as a test, so think about whether that's good for
the language.  I like it, and I think it makes things very simple.

Barry

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 19 Aug 2011 12:42:56 +0100

Hi Barry,

Barry Leiba wrote:

>reviews of convert and imap-sieve.
>
+1.

>I'll be getting to the latter soon.
>
"I've asked you and you ignored me" behavior doesn't seem right in 
Sieve. I don't think any other extension does that.
If we really want to have both (strict conversion and "best effort" 
conversion), then maybe we should have a tagged argument to signal that.

>failed is back to its previous state?
I think I prefer a tagged argument (see above) to control this.

>done, or is it only TRUE if at least one conversion was done?).
>
I think "no conversion is needed" is a success case.

>If it's used only as an action, with no test, then any failure generates
>a runtime error.
>
I don't mind that. But I don't think that that should imply any general 
correlation between actions and tests for all future extensions we might 
design.

---

From: Barry Leiba <[email-masked]>
Date: Mon, 22 Aug 2011 12:42:07 -0400

> "I've asked you and you ignored me" behavior doesn't seem right in Sieve. I
> don't think any other extension does that.

I don't think this is a good reason to have it work this way, unless
we have a use case that merits the extra complexity.  But it might not
matter, if we can agree on my other suggestion, so let's discuss that
below.

>> conversions failed
...
> correlation between actions and tests for all future extensions we might

I don't understand that comment; can you explain it?

Do you mean that making this both an action and a test shouldn't imply
that any other actions can be used as tests also?  I certainly agree:
of course it shouldn't.  Future extensions will define their behaviour
as they like, and might or might not use this model.  This model makes
sense here, I think.  It won't make sense in general, and this
wouldn't be a general change in the behaviour of Sieve actions.

Barry

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 23 Aug 2011 18:38:44 +0100

Barry Leiba wrote:

>we have a use case that merits the extra complexity.
>
Can you clarify what kind of complexity you are talking about?

>But it might not
I tend to agree.

>that any other actions can be used as tests also?
>
Yes. Or rather the other way around: not every test is expected to be 
used as an action. I was thinking about calling functions in C: they can 
be used in tests and they can also be called so that their return value 
is ignored.

>I certainly agree:
Good. Tests and actions are effectively in different namespaces.

---

From: Barry Leiba <[email-masked]>
Date: Tue, 23 Aug 2011 13:47:17 -0400

>>> "I've asked you and you ignored me" behavior doesn't seem right in Sieve.
>>> I don't think any other extension does that.
>>

The complexity of defining a new test to go with the new action.

On the other hand, if we have an action that can also be used as a
test, I think that's simpler.  And it seems we're converging on that.

>>>> Why not have the "convert" action *also* be a test.
...
> Yes. Or rather the other way around: not every test is expected to be used
> as an action. I was thinking about calling functions in C: they can be used
> in tests and they can also be called so that their return value is ignored.

OK.  I think we're good on this, then.  We'll define this particular
action to also function as a test, but I'll put in text that makes it
clear that it's just this case, and that other actions are not
expected to behave this way.

If I don't hear objections from anyone else, I'll go ahead and put out
a new version with this change.

Barry

---

From: Cyrus Daboo <[email-masked]>
Date: Fri, 26 Aug 2011 11:48:42 -0400

Hi Alexey,

> conversion), then maybe we should have a tagged argument to signal that.

Well the redirect action's failure handling is a bit like that - 
implementations get to decide what to do: possibly throw an error, create a 
missing mailbox, file to somewhere else.

I am somewhat nervous about the proposal to have a combined action/test 
command - I'd like to heard from other implementors as to whether this 
represents a significant problem for them or not.

Personally I am OK with taking the redirect approach: error handling is 
implementation defined (could be one of raise an error, ignore failure, try 
an alternative best effort conversion instead).

-- 
Cyrus Daboo

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 26 Aug 2011 17:03:20 +0100

Hi Cyrus,

Cyrus Daboo wrote:

> I am somewhat nervous about the proposal to have a combined 
> action/test command - I'd like to heard from other implementors as to 
> whether this represents a significant problem for them or not.

I thought about that and I decided that there is no problem because 
tests and actions are effectively in different namespaces. In my 
implementation I will need to add code for them in separate places, so 
there is no interference.

---

From: [email-masked]
Date: Sat, 27 Aug 2011 01:14:46 -0700 (PDT)

> missing mailbox, file to somewhere else.

I think you meant fileinto, not redirect, but the point is made regardless.

> represents a significant problem for them or not.

Not sure this matters since we have no intention of implementing convert,
but this is no problem for our implementation. We actually have a couple of
private extensions that work this way already.

				Ned