Repository navigation
FTPFS should use MDTM #456
Description
Activity
Another, probably more complex improvement, would be to cache the LIST response rather than call it for every single file.
On a side note, we have
fs.wrap.cache_directoryfor this, which can be used as a workaround until we figure out the best way to get this working. Here is what I thought about so far, feel free to add comments:Proposed roadmap
1. Introduce a completely new namespace
modifiedthat just contains amodifiedtimestamp.This way we don't break any existing thing, but we keep the namespaces consistent (although I totally understand what you wanted to do with the namespace/subnamespace tuple earlier).
2. Introduce a new meta feature
supports_modified, which marks whether a filesystem supports themodifiednamespace.This is not 100% needed, but just like the
renamemeta feature it would help knowing whether a filesystem supports themodifiedout of the box or not. It is going to beTrueon most filesystems, but will likely help preserving full compatibility with external plugins.3. Make
Info.modifieduse eitherraw["modified"]["modified"]orraw["details"]["modified"]if any of the two is available.This is quite straightforward, yet lets us use MDTM even at the
Infolevel, and not just through thegetmodifiedmethod.4. Patch
getinfoto support themodifiednamespace.We can now add code to fill the
modifiednamespace independently of thedetailsnamespace on filesystems where this is feasible.5. Add a
FS.getmodifiedmethod like you did.The implementation in
fs.base.FS.getmodifiedthen streamlines to:def getmodified(self, path): if self.getmeta().get("modified", False): return self.getinfo(path, namespaces=["modified"]).modified return self.getinfo(path, namespaces=["details"]).modified
6. Patch functions to use the
modifiednamespace where applicable.All the
if_newerfunctions, etc. can be made to usegetmodified.By the way, some thoughts about improving
FTPFSfurther:- We can totally use the same strategy to support the
SIZEFTP command and optimizegetsizeif possible. - FTP servers have
RMFR/RMTOcommands, we could use that forFTPFS.move - Some FTP servers have a
RMDAcommand that we could use forFTPFS.removetree
- We can totally use the same strategy to support the
Sounds good, just got one worry:
We can now add code to fill the modified namespace independently of the details namespace on filesystems where this is feasible.
What happens in filesystems that do not support the "modified" namespace? From what I can tell, your proposal would just fail silently by returning an empty
Infoobject instead of notifying the user with an exception. Otherwise, all 10 implementations ofgetinfoas well as all plugins would have to add an error check for unsupported namespaces.Well, any filesystem that currently supports the
detailsnamespace can be made to support themodifiednamespace, so that's not something I'd worry about. TheFS.getmodifiedimplementation is made aware of this through the call togetmeta. Overall, we should stick to silently ignoring non-existing namespaces to avoid breaking more things.Alright, should be done like you suggested. I'll just have to see how to add useful tests for the new behavior.
Regarding fs.ftpfs.FS.getinfo (
pyfilesystem2/fs/ftpfs.py
Line 648 in 64d7a52
That function tries to use MLST to retrieve arbitrary information of a single file and defaults to LIST if MLST is not supported.
This makes
fs.copy_dir_if_newerincredibly inefficient on systems that do not support MLST.Imo, an easy solution would be to possibly support MDTM as a fallback if only the modified datetime is requested, e.g. if
namespace == ("detail", "modified").Another, probably more complex improvement, would be to cache the LIST response rather than call it for every single file.
I can submit a PR for the first suggestion, if I get the okay that this is something that should be included. Possibly also the second, although I'm not sure yet how to best implement that.