[Twisted-Python] twisted.vfs issues

[Andy Gayton]

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.