Add support for a Task based async API

It'll make the library much easier to use with async/await and fit the modern .NET async friendly way of working.

I'd suggest at minimum the following methods:

public Task<Stream> DownloadFileAsync(string path)
public Task<IEnumerable<SftpFile>> ListDirectoryAsync(string path)
public Task UploadFileAsync(string path, Stream stream, bool overwrite = false)

I've managed to wrap the old IAsyncResult style API you already have for DownloadFile as follows:

await Task.Factory.FromAsync(this.sftpClient.BeginDownloadFile(remoteUri, stream), this.sftpClient.EndDownloadFile) but I'm struggling to figure out the way to do the same for ListDirectory...

I think if SshCommand.EndExecute returned the command, instead of string, as in:

public SshCommand EndExecute(IAsyncResult asyncResult)

then we could do:

public static Task<SshCommand> RunCommandAsync(this SshClient sshClient, string command)
{
    if (sshClient == null)
        throw new ArgumentNullException(nameof(sshClient));

    var sshCmd = sshClient.CreateCommand(command);
    return Task<SshCommand>.Factory.FromAsync(sshCmd.BeginExecute, sshCmd.EndExecute, null);
}

and then await on it.

In the extension method I posted above, if you replace Task<SshCommand> with Task<string> it will work, however, since return type isn't SshCommand we can't examine if the command was a success or failure.

My workaround right now is the following:

public static async Task<SshCommand> RunCommandAsync(this SshClient sshClient, string commandText)
{
    if (sshClient == null)
        throw new ArgumentNullException(nameof(sshClient));

    return await Task.Run(() => sshClient.RunCommand(commandText)).ConfigureAwait(false);
}

Fix sshnet#153 support of Task-based Asynchronous Pattern (TAP) over …

Maybe the library will get the TAP support. If it is not, there is also a nuget package Rehci.SshNet.Async.

I have updated the aforementioned Rehci.SshNet.Async package to also target .NET Standard 1.3 and UAP.

Hopefully it is useful to some of you...

It would be really great to only support async going forward for all operations like getting meta data, listing directories, uploading, downloading and seeing if a file or folder exists.

Fix sshnet#153 support of Task-based Asynchronous Pattern (TAP) over …

The Connect method must also support asynchronous APIs.

The proposed ListDirectoryAsync method could make use of IAsyncEnumerable which is new in C# 8.0, but it requires .NET Standard 2.1 or later.

The proposed ListDirectoryAsync method could make use of IAsyncEnumerable which is new in C# 8.0, but it requires .NET Standard 2.1 or later.

The need of targeting .NET Standard 2.1 is not the biggest impediment for these APIs (cross-compiling could be used for that). The whole library seems to be written without async in mind and requires significant refactoring. All current async methods and wrappers are based on the old Begin/End async pattern, implementation of which turns out to be not really asynchronous and just calls synchronous methods on thread pool threads. This is a well-known antipattern and leads to many problems.

@andriysavin

@andriysavin, I agree regarding SftpClient.
Hovever, SftpSession's BeginX/EndX implementation looks OK to me on first sight.

@drieseng

@drieseng I have committed my attempt at implementing DownloadFileAsync in IgorMilavec/SSH.NET@f510968. Can you please have a look and let me know if this is something you would consider merging into SSH.NET, before I push on with other functions as suggested in this issue?
The implementation is async all the way down, no threads were harmed during implementation of this feature. :)

@drieseng

@drieseng, we are running into the issues with iasyncresult and can use this feature. Is the PR @IgorMilavec completed?

I just added ConnectAsync to the PR. I believe the functionality should now be functionally complete for most uses.

@drieseng

@drieseng can you please have a look at #819? Is it possible to merge this PR and build a beta package? I'm running this code in production on a couple of servers for a month now and did not see any regressions. By building a beta package we would make this more accessible to interested parties and get feedback on code quality and supported usage cases.

@drieseng

@drieseng pinging you again in case you missed the above proposal

@IgorMilavec

@IgorMilavec Sorry man, I've been very busy the last months (or is it years). I'll try to find more time.

Navigation Menu

Repository navigation

Add support for a Task based async API #153

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add support for a Task based async API #153

Description

Activity

omidkrad commented on Mar 30, 2017

omidkrad commented on Mar 30, 2017

bartizan commented on Sep 15, 2017

JohnTheGr8 commented on Oct 12, 2017

niemyjski commented on Feb 23, 2018

wplong11 commented on May 4, 2020

vanillajonathan commented on Feb 22, 2021

andriysavin commented on Feb 23, 2021

IgorMilavec commented on Mar 10, 2021

IgorMilavec commented on Mar 11, 2021

kdcllc commented on May 10, 2021

IgorMilavec commented on May 11, 2021

IgorMilavec commented on Jun 28, 2021

IgorMilavec commented on Aug 18, 2021

drieseng commented on Aug 18, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions