Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Long-Running Actions #368

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Long-Running Actions #368

wants to merge 9 commits into from

Conversation

BerSie
Copy link
Member

@BerSie BerSie commented Feb 7, 2025

To give the UI and the backend a hint that the execution of the resp. operation (action or function) can take a long time. An alternative term definition could be in the UI vocabulary: UI.IsLongRunningOperation.

@RaizoC
Copy link
Contributor

RaizoC commented Feb 7, 2025

What is the intention of this information (and what is meant with "long-running", i.e. when would you rate something as long-running")? Maybe the runtime in case of actions and functions highly depends on the input parameters or the current data in the system or any kind of configuration or even extensions?!
Assuming the annotation is to be used to give the user a hint, that it "might" take a bit longer, I would rather see this in the UI vocabulary and the DataFieldForAction.

@HeikoTheissen
Copy link
Contributor

HeikoTheissen commented Feb 7, 2025

@MarcelWaechter
Copy link
Contributor

Hi,
the idea of this annotation came up for the progress indicator implementation, for which we use a WebSocket connection to show a progress indicator.
The WebSocket is defined through

<Annotations Target="SAP__self.Container">
   <Annotation Term="SAP__common.WebSocketBaseURL" String="webSocketEndPoint" />
   <Annotation Term="SAP__common.WebSocketChannel" Qualifier="progressIndicator" String="progressIndicator" />
</Annotations>

This is a service annotation and is not restricted to specific actions.
To avoid the UI opens a WebSocket connection for every action call, we need an indicator that this action might take long, and it makes sense to open the WebSocket connection to retrieve progress information.

How shall the user experience change for such operations?
Has this been aligned with UX?

As a first version, instead of showing the busy dialog without any additional information, we aim to show a progress indicator, with an optional text on the current backend activity.
Once we support async actions, we'll close the dialog, allowing the user to continue working on the object, and showing the progress in a separate UI part

Shall the request be made with Prefer: respond-async?
Is this related to oasis-tcs/odata-specs#2015?

Not for the first version, but yes this would be the next step

Assuming the annotation is to be used to give the user a hint, that it "might" take a bit longer, I would rather see this in the UI vocabulary and the DataFieldForAction.

Sounds also good to me

@BerSie
Copy link
Member Author

BerSie commented Feb 7, 2025

Assuming the annotation is to be used to give the user a hint, that it "might" take a bit longer, I would rather see this in the UI vocabulary and the DataFieldForAction.

True, therefore I mentioned the alternative approach with UI.IsLongRunningOperation. Currently we have examples for both - IsAIOperation is in UI, and IsActionCritical is in Common. I personally think it's closer to the second one, but as @MarcelWaechter I'm fine with both of the approaches

@HeikoTheissen
Copy link
Contributor

HeikoTheissen commented Feb 7, 2025

To avoid the UI opens a WebSocket connection for every action call, we need an indicator that this action might take long, and it makes sense to open the WebSocket connection to retrieve progress information.

Then I'd suggest to use the WebSocketChannel annotation for that.

<Annotation Term="SAP__common.WebSocketChannel" Qualifier="progressIndicator">
  <String>progressIndicator service.namespace.longRunningAction service.namespace.veryLongRunningAction</String>
</Annotation>

Here progressIndicator is the channel ID and service.namespace.longRunningAction and service.namespace.veryLongRunningAction are the long-running actions.

An annotation on the operation is then not necessary. I will push a commit that contains that suggestion.

@@ -1519,6 +1523,8 @@ followed by URL parameters

&lt;dl&gt;Supported qualifiers and channel IDs:
&lt;dt&gt;`sideEffects` &lt;dd&gt;Notifications about side effects to be triggered by the client (channel ID = non-null annotation value)
&lt;dt&gt;`progressIndicator` &lt;dd&gt;Progress notifications for long-running operations.
The annotation value is a space-separated list of channel IDs which are qualified names of operations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there one channel per operation, or one for all operations? In the latter case, we could specify

Suggested change
The annotation value is a space-separated list of channel IDs which are qualified names of operations.
The annotation value is a space-separated list of the channel ID followed by qualified names of operations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I like your proposal as well. Also added Martin to comment.

At least for now, there is always one channel for all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I will remove the other proposed change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware of the semantic meaning of the WebSocketChannel qualifier, but as we have that, it's a good point to use it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The channel ID must be the service definition name as we have it for events for side effects:
<Annotation Term="SAP__common.WebSocketChannel" Qualifier="progressIndicator">
  <String>i_salesoder_sd service.namespace.longRunningAction service.namespace.veryLongRunningAction</String>
</Annotation>
  1. I'm not sure from where the names of the long running actions should be derived. Do we expect that there will by a corresponding (ABAP) CDS annotation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we proceed here? We have now a very technical annotation (that doesn't describe any semantics but the technical means) - are we now thinking about how we make it (semantically) "understandable" for application developers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "Progress notifications for long-running operations" is not sufficient, you can add documentation after

The annotation value is a space-separated list of the channel ID = service definition name followed by qualified names of operations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second question from @martin4all tells me that it's hard to decide for a framework, which action has to be put there. That's why the app developer has to do that. And I can imagine that an annotation term "WebSocketChannel" is nothing a business app developer would expect, because it describes a technical solution not a semantic meaning/description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be a CDS annotation on the operation, which is translated into the OData annotation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True ... and as long as we don't call it UI.IsLongRunningAction I'm fine with that :-)

@HeikoTheissen
Copy link
Contributor

HeikoTheissen commented Feb 12, 2025

What do you think about the following alternative?

<Annotation Term="SAP__common.WebSocketChannel" Qualifier="progressIndicator" String="i_salesoder_sd"/>
<Annotation Term="SAP__common.WebSocketRelatesTo" Qualifier="progressIndicator">
  <Collection>
    <String>service.namespace.longRunningAction</String>
    <String>service.namespace.veryLongRunningAction</String>
  </Collection>
</Annotation>

Here the qualified names of the operations related to the web socket channel are annotated separately. Perhaps this is easier to generate for @martin4all.

@RaizoC
Copy link
Contributor

RaizoC commented Feb 12, 2025

I do not like "longrunning" as it is completely misleading and out of context. The topic is about if an action (or in general any operation) may fire a progress or not. If we want to control the same, i.e. the client want to know if it makes sense to e.g. open the web socket, then we need to annotate the same. Not sure this information should be bound to the web socket as the server may independent of client send a progress and if no one is interested nothing would happen. And even if the server even stated that a progress can be sent or is sent, this does not imply the client also wants to consume it.

IMHO the move from the UI domain should not have been done, but the term proposed was misleading. I agree to @MarcelWaechter to rather add this into the DataFieldForAction if a progress is to be shown. And if application want it for shorter actions, why not. It cannot be defined statically how long an operation finally runs.

@HeikoTheissen
Copy link
Contributor

HeikoTheissen commented Feb 12, 2025

At any rate we need a Common.WebSocketChannel annotation (which in turn requires a Common.WebSocketBaseURL annotation). Otherwise we would violate the concept that we have introduced for the sideEffects channel.

The remaining question is whether we annotate the relevant operations in Common.WebSocketRelatesTo (as proposed in #368 (comment)) or in the UI vocabulary.

@MarcelWaechter
Copy link
Contributor

So yes, the WebSocket channel is needed.
I'm actually fine with both...
Using this websocket channel to mention actions that provide a progress, why not - would not mention something with long running, but just focus on "providing progress indicator via WebSocket"

if we go with an annotation in the UI vocabulary, in the datafield, how would we call it if we don't want it be called long running? but just saying it might show a progress indicator, and still needs the websocket channel?
I could imagine it's hard to find a good name here?

@BerSie didn't accept my proposal:
UI.ThisActionIsExpectedToTakeAnUnreasonablyLongAmountOfTimeSoYouMightWantToConsiderDisplayingSomeSortOfProgressIndicatorToKeepTheUserFromGoingAbsolutelyInsaneOrWorseThinkingTheAppHasCrashedAndForceClosingIt
😆

@BerSie
Copy link
Member Author

BerSie commented Feb 13, 2025

if we go with an annotation in the UI vocabulary, in the datafield, how would we call it if we don't want it be called long running? but just saying it might show a progress indicator, and still needs the websocket channel?
I could imagine it's hard to find a good name here?

UI.IsActionReportingProgress ?

@HeikoTheissen
Copy link
Contributor

UI.IsActionReportingProgress

It is not only for actions but also for functions. What about UI.ReportsProgress?

@BerSie
Copy link
Member Author

BerSie commented Feb 13, 2025

UI.IsActionReportingProgress

It is not only for actions but also for functions. What about UI.ReportsProgress?

OK - replace Action with Operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants