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

New implementation for accessing content on FTP servers #12

Open
Arlodotexe opened this issue Jan 24, 2023 · 4 comments
Open

New implementation for accessing content on FTP servers #12

Arlodotexe opened this issue Jan 24, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@Arlodotexe
Copy link
Owner

Arlodotexe commented Jan 24, 2023

Feature proposal

An implementation of OwlCore.Storage that's backed by FTP / SFTP.

We should use FluentFTP to do this, since System.Net.FtpWebRequest is on life support.

Taking a dependency means we'll need to create a new repo and package for this, but this library has all the right features, it's worth it. Support for IAsyncEnumerable will match up perfectly with IFolder and general Task/async support will keep things responsive (not all FTP libraries support async).

When implementing this, be sure that there are separate implementations for folders that are modifiable and non-modifiable.

Additional info

This is needed for in order to download the MusicBrainz catalog programatically, so we can create the tooling needed to port their database dumps to IPLD / IPFS using automation.

See our discussion on Discord in the UWP Community.

We may also look into using this support FTP functionality in the Files app.

Finally, we'll use this in Strix Music to add support for playing music from FTP sources.

@itsWindows11
Copy link
Contributor

itsWindows11 commented Jun 11, 2024

FTP and SFTP should be completely different implementations, as SFTP isn't the same as FTPS.

SSH.NET is basically the only actively maintained free SFTP/SCP library that I could find. Problem is it has some methods that are sync only, and others that are async but use the old Begin/End pattern, or async with callbacks, and finally async using Task, and it doesn't support checking the creation date of a file.

Instead of potentially exhausting the threadpool for IO if one were to stress test the SFTP implementation using that library, we might need a fork that properly makes use of Task and async, and maybe contribute back to the original repo.

We might need to look into adding progress reporting too in the abstraction if it already isn't there, Files might need it.

@Arlodotexe
Copy link
Owner Author

Arlodotexe commented Jun 11, 2024

👋 Thanks for the clarification @itsWindows11. SFTP and SCP are indeed a separate protocol from FTP. We'd want to use a separate library, SSH.NET, to implement these with the storage abstraction.

Following our existing name schemes (e.g., OwlCore.Storage.SharpCompress), these would end up in two different packages: OwlCore.Storage.SSH.NET and OwlCore.Storage.FluentFTP.

This ticket is for FTP and FTPS, which is what FluentFTP would be used for. For SCP and SFTP, we should make a separate ticket to track the contributions we need to make to the upstream library. They have some pending PRs on async that could get us started.


Progress reporting is a good call out as well. Generally the library is built so that progress reporting can be implemented application-side during an operation (e.g. navigating directories, moving things around, stream transfers). However, there's a few long-running extension methods where we could consider an IProgress parameter, such as Copy and Move.

Here is the inbox implementation for copying a file into a folder. Presumably, dotnet should supply us with an overload that takes an IProgress in Stream.CopyToAsync, but that doesn't appear to be the case. We'd need to build this ourselves or use one of the many versions that others have built.

Moving progress reporting to a separate issue, we can tackle this there.

@0x5bfa
Copy link

0x5bfa commented Jun 12, 2024

We might need to look into adding progress reporting too in the abstraction if it already isn't there, Files might need it.

Definitely true. That’s because we did so for SevenZipSharp to support progress report by forking to our org and modifying.

@itsWindows11
Copy link
Contributor

This issue should be fully resolved now that we have the implementation, still awaiting ownership to be transferred and the package to be published though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants