w3c / wot-scripting-api

Web of Things (WoT) Scripting API
http://w3c.github.io/wot-scripting-api/
Other
42 stars 28 forks source link

New op values (cancelaction, queryaction,and queryallactions) #335

Open danielpeintner opened 3 years ago

danielpeintner commented 3 years ago

There is a WIP PR, see https://github.com/w3c/wot-thing-description/pull/1208

Once baked into the TD document we might need to discuss the consequences for the Scripting API.

danielpeintner commented 2 years ago

I had a chat with @egekorkan and I would like to describe how he sees the relation between TD and Scripting API w.r.t. cancelling/querying actions.

I'm looking forward to comments! @egekorkan please comment if I misinterpreted something 🙃

egekorkan commented 2 years ago

Thanks for noting this down! I think it represents my thoughts really well. I had opened https://github.com/w3c/wot-thing-description/issues/1408 after this discussion. Just to avoid some misconceptions:

zolkis commented 2 years ago

The "async" terminology is not correct, as all actions are async (on the network). A better name is needed. Basically we want to express whether an action is cancellable and queryable, which is a better split than inventing a term like "async" which is overloaded on its own, and here it means two different things under the same hat - that is a mess. Also, there might be cases when an action is cancelable, but not queryable, and when is queryable, but not cancelable. WoT doesn't dictate a new standard here, but models other protocols, right? So we need to be able to model those combinations, instead of sweeping them under the same rug.

As for scripting API design, the action id would be a new thing, but if we want separate op's for queryaction and cancelaction, then the id is needed, and we should model it by separate calls indeed. The reason is the following (discussing design alternatives).

  1. From an idiomatic point of view, we could also use AbortController and AbortSignal to cancel, but we'd still need a method for the status query, so this design would not be clean, nor tracking the TD op's well.

  2. As to the idea of an action control object with 3 methods,

    • query() or Promise<ActionStatus> status(options),
    • cancel() and
    • Promise<InteractionOutput> result().

If a script doesn't want to cancel or query the status, could right away call result = await action.result();

But then the same result() method should be used for the "sync" type of action, which is a complication.

  1. Alternatively, variable return types (action control object or InteractionOutput) would be a very ugly kludge, so no-no.

In summary, yes, separate methods + id seem to be the cleanest design. Scripting spec can add support for action id (a breaking change) and 2 new methods for queryAction() and cancelAction() (non-breaking). Scripting spec doesn't need to expose the TD flag for "async", since if an action is not cancelable, the cancel() method could fail with an appropriate error.

egekorkan commented 2 years ago

I would be open for other names. Just a precision on the following comment:

Basically we want to express whether an action is cancellable and queryable, which is a better split than inventing a term like "async" which is overloaded on its own

An action can be queried even if it is synchronous. It is relatively easy to implement a 10 second lasting response handler in a backend library so the consumer can query it in the meantime. The async flag does not indicate whether an action is queriable or not.

zolkis commented 2 years ago

Even if an action appears to be synchronous, it is asynchronous, and if it can be queried, just proves it's async. :). So you're saying all actions are queryable, regardless if they are labeled "sync" or "async"? Then the only thing needed is whether an action is cancellable or not. Since cancellation in general cannot be guaranteed, we don't even need the cancellable information, since an attempt to cancel can be make even if the action doesn't support it - then it (the Thing in the other end, or the local scripting implementation) will either signal an error, or ack, or nothing.

egekorkan commented 2 years ago

Sort of but I think before future discussion, what I mean by these words is what I have illustrated (literally) at https://github.com/w3c/wot-thing-description/issues/890 . I would be open for alternatives

zolkis commented 2 years ago

On the call on 7. March several use cases for actions were discussed:

In addition to the use cases, various policies are possible as well, for instance:

@danielpeintner @ashimura @egekorkan @relu91 @JKRhb @k-toumura please complete/edit these (edit this comment freely).

zolkis commented 2 years ago

The question is, can we design an API that would support all these variations in use cases and policies.

Generic alternatives were discussed above, were the conclusion was we need the design with separate cancelAction() and queryAction() calls.

However, we could extend invoking an action with more options, and support action id's, even if the TD spec doesn't (yet) support them (since the implementations can always generate an id).

Instead of returning an id, we could also use a returned object instead, which is more future-proof. For start, the updated API could look something like

Promise<ActionControl> invokeAction(DOMString actionName,  // could also return a tuple
                              optional InteractionInput params = null,
                              optional InteractionOptions options = null);

Promise<undefined> cancelAction(DOMString actionName,
                              optional InteractionInput params = null,  // contains the id
                              optional InteractionOptions options = null);  // or data could contain the id 

Promise<InteractionOutput> queryAction(DOMString actionName,
                              optional InteractionInput params = null,  // contains the id
                              optional InteractionOptions options = null);  // or data could contain the id 

[SecureContext, Exposed=(Window,Worker)]
interface ActionControl {
    readonly attribute Promise<InteractionOutput> result;
    readonly attribute USVString id;
};

Alternatively, invokeAction() could return a tuple of (Promise , USVString), but I thought an object would be more future-proof.

Note that the invokeAction promise can resolve right away, whereas the client would mainly await for the result promise.

const ac = await thing.invokeAction("temperature", { units: "Celsius" });
const id = ac.id;
var output = await ac.result;

or directly

var output = await (await thing.invokeAction("temperature", { units: "Celsius" })).result;
egekorkan commented 2 years ago

I like the proposal. Just that I think it might be too early to define something like a result if it really should signify the end of the physical action process. We do not have any plan on how to map the result of a query to the state of the action, thus we do not know if pending means something else than completed

danielpeintner commented 2 years ago

2 comments (which do not affect the proposal itself)

danielpeintner commented 2 years ago

FYI: besides queryaction and cancelaction there is also queryallactions (I updated the title of the issue accordingly)

danielpeintner commented 9 months ago

I know that cancelaction, queryaction etc are defined in the TD specification but I still have the feeling the "way it works" is under-specified. Having said that, I don't think it is implementable that way.

Do you see it differently @egekorkan ? see also https://github.com/w3c/wot-thing-description/issues/1408

Hence I propose to add the label:wait-for-td

egekorkan commented 9 months ago

I agree that it is underspecified and we can indeed wait for the TD TF to resolve this first. The parts that are underspecified are mentioned above but to summarise:

Of course once those are specified by the TD, the way they are exposed in the scripting level needs a separate discussion.

danielpeintner commented 8 months ago

The corresponding TD issue is https://github.com/w3c/wot-thing-description/issues/1408. I added the label "Needed by other TF"