Add async in the mix

[jairbubbles]

It would be nice to have an async API. The goal of this discussion is to discuss the best strategy.

Here are some initial thoughts:

• Should we add a sync and async client? Like Releases and ReleasesAsync ?
• Duplicate methods in the existing clients? Like Get and GetAsync (bonus question : what do when the operator [] is used?)
• We cannot break compatibility so replacing the sync API is not an option

[meziantou]

The return type for collection cannot be IEnumerable<T> but IAsyncEnumerable<T>. I think a good strategy would be to create a PagedResponse<T> type which implements the right interfaces such as this one. All new methods would use this return type instead of IEnumerable<T>.

Should we use HttpClient instead of the old WebRequest? This has some implication on the public API such as virtual GetResponse (code) that won't be used by the async path. But it would provide good feature thanks to HttpHandler (Polly, Logging, etc.)

Should we add a sync and async client? Like Releases and ReleasesAsync ?

There are at least 2 strategies:

• Adding async methods to the existing interfaces
• Adding new interfaces

I do prefer the first option as it's easier to discover new methods. Also analyzers may suggest to use the async equivalent when possible.

bonus question : what do when the operator [] is used?

These operators won't exist in the async version. Thes accessor should be replaced by a method with at least 2 arguments because of the CancellationToken.

[jairbubbles]

I do prefer too to have them on the same interface, it will also allow to add them progressively.

[meziantou]

Using WebRequest async methods (GetResponseAsync) look like the simplest way to support async methods. However, this doesn't support CancellationToken :( It may be possible to do it by ourself using CancellationToken.Register and the Abort method. We would also need to provide a new virtual method RequestOptions.GetResponseAsync() to provide extensibility.

However, we cannot take advantages of nice features such as DelegateHandler.

[meziantou]

I made a small prototype of the async implementation. It is not ready to be merged yet.
There is one point I don't like is adding new methods to return a collection (Get => GetAsync) as the method is not really async. Only the enumeration is... (https://github.com/ubisoft/NGitLab/pull/137/files#r754369850)

Maybe you can check the PR I tell me what you think about it? #137