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

Add AsyncTaskCodeActivity type #145

Closed
Foxtrek64 opened this issue Apr 27, 2021 · 2 comments · Fixed by #149
Closed

Add AsyncTaskCodeActivity type #145

Foxtrek64 opened this issue Apr 27, 2021 · 2 comments · Fixed by #149

Comments

@Foxtrek64
Copy link
Contributor

Foxtrek64 commented Apr 27, 2021

Currently for those wishing to build a custom asynchronous activity, one must implement AsyncCodeActivity and work with the NetFx 3 async library, implementing BeginExecute and EndExecute. For many newer developers, this model is foreign and may be difficult to find adequate documentation of how to implement an activity using this model.

I would like to propose a new type, AsyncTaskCodeActivity, which instead exposes an abstract ExecuteAsync() method which returns a Task or Task<T> as appropriate. This can reuse much of the existing code available in the Windows Workflow Foundation library, such as AsyncCodeActivityContext.

Proposed API Changes

public abstract class AsyncTaskCodeActivity : AsyncCodeActivity
{
    protected abstract Task ExecuteAsync(AsyncCodeActivityContext context, CancellationToken cancellationToken);
}
public abstract class AsyncTaskCodeActivity<TValue> : AsyncCodeActivity<TValue>
{
    protected abstract Task<TValue> ExecuteAsync(AsyncCodeActivityContext context, CancellationToken cancellationToken);
}

Considerations/changes:

  1. Should System.ValueTask be used instead of System.Task? If a user wraps a MemoryStream or some other existing ValueTask that can return synchronously, ValueTask allows us to wrap the return value and avoid allocating. There are however a couple of potential downsides:
    • ValueTask/ValueTask<T> is less usable than its Task/Task<T> counterpart. However, since ValueTask/ValueTask<T> is best used in environments where it is simply going to be awaited, this makes it a particularly good candidate for UiPath. If it is being called outside of the UiPath code runner, then it is likely being done in a unit test by the developer who is building the activity. It will be on them to handle it appropriately.
    • ValueTask/ValueTask<T> is larger than Task/Task<T> when wrapping Task/Task<T>. This is a measurable difference in microbenchmarks, however for utilization in an environment like UiPath, I doubt it will make a big difference.
  2. Is AsyncCodeActivity the correct target for extension? Should it instead extend Activity like AsyncCodeActivity does? The way my current implementation of AsyncTaskCodeActivity works, uses BeginExecute to start the task and uses a TaskCompletionSource to track the lifetime of the task. Then it uses EndExecute to return the value of the task. In the case of AsyncCodeActivity (without a return type), it uses an empty VoidResult struct as its result (since I can't use void as a return type). This type works similarly to MediatR's Unit type.

Should there be an interest in this change, I would be happy to submit a PR.

@lbargaoanu
Copy link
Contributor

lbargaoanu commented Apr 28, 2021

Sounds good! A PR is welcome.

  1. We're not a high performance library, so Task seems best.
  2. It seems there's useful stuff to reuse in AsyncCodeActivity. But make sure to seal BeginExecute/EndExecute. Also see this.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants