Open ReimarBauer opened 11 months ago
Before working on this we should talk about if we want to keep Pyfilesystem2 in our lib or replace it completly by os / pathlib.
When we introduced FS we had anything using it. Over the years it gots a mixup. At the time we introduced it, there was no pathlib and we also had problems with tests on windows because of path. The idea that we use it with FS pathes seems not become the standard. We can use it with our nextcloud but we don't. etc. Also with mscolab we can do things differently, e.g. exchange images, or config over chat.
Let us collect pro's and con's
https://github.com/Open-MSS/MSS/pull/2405/files#r1665854420
when we want to keep fs in the future we can't use send_from_directory. We have to replace it by send_file because this can also send a BytesIO object
But this also needs an intermediate step, reading the fs data into a BytesIO object.
If we want to keep using fs and use it properly we can't use any function that expects a path-like object. send_from_directory
is just one of them. This stops us from e.g. using any path sanitization features that flask does automatically, among other things.
What is the use case for fs, at least on the server side, anyway?
But this also needs an intermediate step, reading the fs data into a BytesIO object.
Not really, what fs.open
returns is a file-like object which should already be useable as a BytesIO-like object (if opened in bytes mode). So it would just need a with:
statement opening the file, before sending it. Again, path sanitization would be on us, then.
What is the use case for fs, at least on the server side, anyway?
In the past there was no file storage on Google Cloud Platform. It is an option to deploy on such platforms and to manage the file store over the network.
When I look on the Release history on PyPi, https://pypi.org/project/fs/#history
More than 2 years ago 2.4.16 was released. Also on https://github.com/PyFilesystem/pyfilesystem2 it seems not further developed. There are some PR open, it seems not that maintained. It wasn't added to the standardlibrary. (So that others would care now)
For me, that's enough to deprecate its use in our projects
on server we should have os.path. on ui we should have fs.path
fs.path means we can use pyfilesystem urls e.g. webdavfs or sshfs urls mapped to a "fs"path. That means our msui_settings.json can come from a nextcloud user dir and we can save images to an fs url too.
os.path is only capable to store on local or mounted filesystems.
if ui needs an os path then it gets converted by .getsyspath()