-
-
Notifications
You must be signed in to change notification settings - Fork 933
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
Adds DownloadFileAsync, UploadFileAsync and SynchronizeDirectoriesAsync overloads #1515
base: develop
Are you sure you want to change the base?
Conversation
That failing test seems to flaky, its failing also on other pull requests... |
Hi @Havunen I am definitely in favour of adding these overloads, but I think they need to have functional cancellation support with a Secondly, do you know why the tests were failing in the first commit? I think these ought to be investigated before taking the change. Lastly (once the bigger problems are resolved), the optional parameters relating to task factory/creation etc. should be removed. These would expose details of the implementation which would not be desirable. Let me know what you think |
Yeah I changed them to async - await but realized they were testing something while the upload was in progress so I reverted those |
For that we maybe we need to create internal implementation for the methods so the cancellation can be checked there and then synchronous API has that async value as null |
Any update on this issue? |
I have been busy with all other work ongoing. Maybe somebody else can also add the Cancellation token support, it should not be too difficult there are already some methods using it. |
Hi!
It would be awesome if SSH.NET package could provide these few async overload methods for more ergonomic developer experience. Previously there were implemented here https://github.com/JohnTheGr8/Renci.SshNet.Async but that package has not been updated for a while. JohnTheGr8/Renci.SshNet.Async#17
Hopefully this could be merged! Thanks!