James Y Knight wrote:
So, starting to look through twisted.vfs, I'm finding a few things that need work.
Hey James, Thanks for the feedback. We need it. Heaps of decisions for the vfs stuff have been put off to see what other use cases would need from the vfs. Inparticular permissions and metadata.
1) I see no way of reading from or writing to a file in ivfs.IFileSystemLeaf.
The vfs stuff is still heavily influenced by the interface that conch expects as sftp has been the main motivation for the current contributors. Reading and writing is done through writeChunk and readChunk - we've always felt this wasn't quite right though for a general backend. But after two sprints we still haven't come up with something that is better. Adding the web2.Stream adaptor seems to have glazed over the issue for protocols that read/writeChunk doesn't work for. Spiv even used streams for the vfs ftp adaptor! I've added read/writeChunk to ivfs.IFileSystemLeaf's interface.
2) createFile is racy -- it requires opening a file by the given name, with default permissions, then immediately closing it.
:), racy is good right?
In addition, it doesn't specify whether it's an error if the file already exists.
It should, I've added this to the interface.
3) Looks like all operations are blocking? What about a remote vfs? I think every operation in the vfs interface ought to be non-blocking.
The other option is the vfs interface could be maybe deferred. Most protocols are good at handling this (sftp, streams). But given how easy it is to return deferred.succeed - it's probably simpler to say always non-blocking.
4) IFileSystemNode.remove doesn't say whether it's a recursive delete (on a directory)
Hrm yeah - should it? Or should this be handled by higher level utilities (eg shutil). The current os backend uses os.rmdir, so doesn't do a recursive delete. I've updated the interface to say that it doesn't.
, and .rename don't specify whether newName can be in a different directory, whether it replaces an existing file, or whether it works on a directory.
The method is against Node, so it works on directories. This is os.rename's spec: --- Rename the file or directory src to dst. If dst is a directory, OSError will be raised. On Unix, if dst exists and is a file, it will be removed silently if the user has permission. The operation may fail on some Unix flavors if src and dst are on different filesystems. If successful, the renaming will be an atomic operation (this is a POSIX requirement). On Windows, if dst already exists, OSError will be raised even if it is a file; there may be no way to implement an atomic rename when dst names an existing file. Availability: Macintosh, Unix, Windows. --- Should vfs be aiming to provide consistent behaviour for all operations across all backends? Or should some behaviour be left down to the particular backend to decide? For the moment I've updated the interface to read: Renames this node to newName. newName can be in a different directory. If the destination is an existing directory, an error will be raised.
5) Errors are coarse-grained. Everything is a VFSError, and the only detailed information is in human-readable text, not any nice computer- readable form.
yeah :( that needs to be fixed.
6) Need some support in the interface for extended attributes.
There's getMetadata. That let's you return arbitrary attributes. Would that cover what you're thinking? Protocol's should try to get by with as little metadata as they can. If a backend doesn't supply a bit of metadata a protocol must have, then it won't be able to be used with the protocol. Andy.