Implement async/await approach

[Zlata-Zueva]

Imlements async/await approach using System.Threading.Tasks. All methods are renamed accordingly, adding Async postfix to indicate that they implement async/await approach.

[stanislav-bedunkevich]

Looks good, thanks!

[larf311]

Is it idiomatic in C# to append Async to all the method names? I would think the method signature would be sufficient to indicate that it is async.

[NRHelmi]

Is there a reason to make all client functions async ? If so let's keep the original functions names otherwise I think Execute (maybe ExecuteV1) is the only function that needs to be asynchronous.

[larf311]

@NRHelmi https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/. It's the idiomatic way of dealing with any network requests. Kind of like a Future in Java or Promise (or async/await) in JS.

[stanislav-bedunkevich]

Yeah, it is @larf311 @NRHelmi. For example, AWS SDK follows the same approach:
https://github.com/aws/aws-sdk-net/search?p=6&q=Async+Task

[stanislav-bedunkevich]

Is there a reason to make all client functions async ? If so let's keep the original functions names otherwise I think Execute (maybe ExecuteV1) is the only function that needs to be asynchronous.

That is because essentially all SDK methods do end up doing an HTTP request. Now we need to make it async so that the thread management part of .Net could take care of those threads for us and be less burdened. The link Trevor posted above also describes that in more detail.

[NRHelmi]

Is there a reason to make all client functions async ? If so let's keep the original functions names otherwise I think Execute (maybe ExecuteV1) is the only function that needs to be asynchronous.

That is because essentially all SDK methods do end up doing an HTTP request. Now we need to make it async so that thread management could take care of those threads for us and be less burdened. The link Trevor posted above also describes that in more detail.

Fair enough, is it also required to have all async function renamed to *Async ?

[stanislav-bedunkevich]

Is there a reason to make all client functions async ? If so let's keep the original functions names otherwise I think Execute (maybe ExecuteV1) is the only function that needs to be asynchronous.

That is because essentially all SDK methods do end up doing an HTTP request. Now we need to make it async so that thread management could take care of those threads for us and be less burdened. The link Trevor posted above also describes that in more detail.

Fair enough, is it also required to have all async function renamed to *Async ?

Well, it's the common practice in the C# world :) I added a link to the AWS SDK where they also stick to the same approach, even though Java SDK may not have the Async postfixes.

I am not sure we need to have the method names exactly matching across all of the SDKs, as some languages have practices like this one and we'll likely to end up deviating from that. Moreover, we are already kind of, if we consider the various PascalCase, snake_case and camelCase diffs we have in place :)

[NRHelmi]

LGTM thanks ❤️

[larf311]

I think the AWS SDK has *Async b/c it also has non-async versions. That being said, the Google BigQuery API also does the same thing (offers both sync & async) so I guess we should do it in case we end up offering both.

[vilterp]

Ok, so for the execute functions, we renamed

• Execute => ExecuteWaitAsync (poll until done)
• ExecuteAsync => still ExecuteAsync? (return immediately)

Fair enough 🤔 Better than ExecuteAsyncAsync 😅

[stanislav-bedunkevich]

@larf311 yeah, it seems they both do, right. Although the sync version is usually a wrap around its async version like:

        public IDownloadProgress Download(string url, Stream stream)
        {
            return DownloadCoreAsync(url, stream, CancellationToken.None).Result;
        }

        /// <inheritdoc/>
        public async Task<IDownloadProgress> DownloadAsync(string url, Stream stream,
            CancellationToken cancellationToken)
        {
            return await DownloadCoreAsync(url, stream, cancellationToken).ConfigureAwait(false);
        }

I agree if we need to provide a sync version later for all of the methods in SDK it would be easier done having the async versions in place (this PR). I am kind of unsure we need that though, as users can write Client.DoSomethingAsync(...).Result and make it sync.

Do you have any objections against merging this PR as it is now?

[larf311]

No, looks good. 👍