On Mon, Sep 26, 2005 at 03:26:46PM +0100, Alexey Melnikov wrote:
Hi folks,
I took a stab at updating draft-martin-sieve-notify-01.txt, which should
be published soon as draft-ietf-sieve-notify-00.txt. The document is
attached.
Hi-
On my first read through, I had the idea that it was defining an action
"notify" that would itself generate a notification, deferred until the
end of the script's execution (otherwise the "denotify" made no sense).
After reading this thread and having another go or two at the document,
I now get the idea that "notify" merely enables notifications to be sent
when *other* actions are taken. If that's the correct reading of the
draft, then I think it needs to be made clearer that the "notify" verb
merely enables notifications, and the notifications are triggered by
other actions. I'm still not sure I have interpreted it correctly.
Also it's not clear if a single notification (one per :id, presumably)
is sent that summarizes all actions that have been taken, or if one
notification must be sent per action (per :id). Nor is it clear (to me)
whether "denotify" cancels any queued-up notifications, or whether it
merely disarms the trigger (and queued-up notifications as a result of
already-executed actions are still sent). And what happens if you've
executed two "notify" statements with different ":id"s -- does each
subsequent action then get two notifications? That seems reasonable but
probably should be stated.
It may just be me, of course, but I think some of this could be a lot
clearer.
As for a few specific comments (pared down a little):
> 1. Introduction
>
> This is an extension to the Sieve language defined by [SIEVE] for
> providing instant notifications of sieve actions that have been
> preformed. It defines the new action "notify".
That definition suggests that executing the "notify" action will cause
a notification.
> 1.0
>
> Sieve interpreters for which notifications are impractical or is not
> possible SHOULD ignore this extension.
What does this mean... are you recommending that interpreters implement
the extension as no-ops, so that the "require" will work but the actual
notify statements will not? I mean, this seems to be recommending
something other than the usual thing of having the "require" fail to
operate if the capability is not available. If all this means is
"have the 'require' fail if the capability isn't available" -- doesn't
that go without saying?
> 3.1. Notify action
> ... If an URI schema is
> specified that the implementation does not support, the notification
> MUST be ignored. An implementation treats this as a warning
> condition and execution of the sieve script MUST continue.
I find "must be ignored" and "treat as a warning condition" to be
contradictory.. Perhaps the intent is "must not be treated as a fatal
error, may result in a warning, but the script must continue"
> Some notification methods allow users to specify their
> state of activity (for example "busy" or "away from keyboard"). If
> the notification method provides this information it SHOULD be used
> to selectively send notifications. If, for example, the user marks
> herself as "busy", an implementation SHOULD NOT send a notification
> for a new mailing list message with a priority of :low, however the
> user should be notified of a high priority action. If the
> notification method allows users to filter messages based upon
> certain parameters in the message, users should be able to filter
> based upon priority. If the notification method does not support
> priority, then this parameter MUST be ignored.
Is any of this in scope? That stuff sounds to me more like details of
the way the specific notification platform operates than how the Sieve
implementation should operate. It would be pretty hairy for the Sieve
implementation to figure out what user to query, even assuming the
notification system directs the message to a particular user, let alone
how to find out what the user's state is. Then again, I may be
misunderstanding.
> 3.2. Denotify Action
>
> Usage: denotify [MATCH-TYPE string] [<":low" / ":normal" / ":high">]
I don't care for the fact that "match-type" takes an argument; this is
different from every other context for "match-type." I would much
prefer ":id" with an argument to specify the id(s) to affect, as it's
already defined with "notify," and revert "match-type" the way it is
everywhere else.
For that matter, rather than two verbs "notify" and "denotify" I'd
rather see just one verb, with modifiers to toggle the notification on
or off and/or to cancel. e.g.:
notify :cancel # to cancel any pent-up notifications
notify :off # to disable notifications for future actions
notify :on # to [re]enable a notification
> 4. Interaction with Other Sieve Actions
>
> Notifications MUST be sent in all cases, unless a reject action is
> also executed. Users may wish to be notified of a message being
> discarded, for example.
Might users not want to be notified of a 'reject' action too? Why not
let them have that control? The :off/:on/:cancel thing would facilitate
that.
> The notify action is compatible with itself.
Meaning?
With apologies for any extra denseness,
mm