Opened 7 years ago

Closed 4 years ago

#2815 enhancement closed wontfix (wontfix)

Update VFS backends to an async interface

Reported by: cablehead Owned by: cablehead
Priority: highest Milestone:
Component: vfs Keywords:
Cc: Branch: branches/vfs-async-backends-2815
(diff, github, buildbot, log)
Author: andy Launchpad Bug:

Description

Manually reapply the changes from source:/branches/makeVFSAsync-1223-2 to trunk once #1264 is merged.

Give the changes their own name space, so the adapters can be incrementally upgraded to the new backends.

Change History (11)

comment:1 Changed 7 years ago by cablehead

  • Status changed from new to assigned

comment:2 Changed 7 years ago by andy

  • author set to andy
  • Branch set to branches/vfs-async-backends-2815

(In [21838]) Branching to 'vfs-async-backends-2815'

comment:3 Changed 7 years ago by cablehead

This ticket is part of a group to incrementally refactor twisted.vfs to use an async interface. See #1223 for the summary of tickets.

comment:4 Changed 7 years ago by cablehead

I've put the new backends in twisted.vfs.newbackends.

We need to keep the old sync backends around until all adapters have been switch over to the async interface.

This is the target layout I'm aiming for once everything is converted and we can clean up (#2819):

twisted/vfs/__init__.py
twisted/vfs/ivfs.py
twisted/vfs/backends.py
twisted/vfs/adapters.py
twisted/vfs/plugin.py

twisted/vfs/test/test_backends.py
twisted/vfs/test/test_adapters.py
twisted/vfs/test/test_decorator.py
twisted/vfs/test/test_plugin.py

comment:5 Changed 7 years ago by cablehead

  • Keywords review added
  • Owner cablehead deleted
  • Priority changed from normal to highest
  • Status changed from assigned to new

comment:6 Changed 7 years ago by exarkun

  • Owner set to exarkun
  • Status changed from new to assigned

comment:7 Changed 7 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to cablehead
  • Status changed from assigned to new
  • twisted/vfs/__init__.py
    • copyright date should be something other than what it is, not sure what
  • twisted/vfs/ivfs.py
    • docstring formatting changes are going in the wrong direction
    • What these interfaces should actually say is still an open question, I think. This would probably benefit from some real-time conversation with various interested parties involved. You've probably already noticed IFTPShell which is one take on this interface (it's not exactly the same shape, but many of the ideas apply here). There's also AsynchronousFileInputOutput which is incomplete and possibly incorrect, but something worth including in the discussion.
  • twisted/vfs/decorator.py - dunno about this :)
  • twisted/vfs/newbackends.py - is there a reason this code can't just go into twisted/vfs/backends/inmem.py? If we want to promote it one level, twisted/vfs/backends/__init__.py can import it (and once we get rid of everything else in backends/, we can get rid of it and move backends/__init__.py to backends.py)

comment:8 Changed 6 years ago by cablehead

  • Keywords review added
  • Owner changed from cablehead to exarkun
  • updated copyright date on files touched in this branch
  • updated doctrings in ivfs to meet standards
  • decorator.py helps to handle rewrapping class instances for decoratored objects which create new instances of themselves. E.g, File has a method, getChildFile -- BeDifferent(File()).getChildFile() <- child fild needs to be made different.
  • i had a shot at promoting vfs.decorator to python.decorator. I agree it's hairy, and the usage in vfs doesn't really warrant introducing something like this. It should be considered generally useful to deserve inclusion. It would benefit from refinement especially before being made a pulic API. At the moment its still under twisted.vfs and I appreciate its heavy handed for one usage.
  • twisted.vfs.backends contains all of the implemented backends - so it doesn't really go in inmem.py. I've moved it to init.py with the aim to move it up to vfs/backends.py once we get to #2819
  • the original draft of the vfs interface was heavily influenced by conch's interfaces - and that's were writeChunk and readChunk come from
  • i've quit my job in LA that's sucked up all my life for the last 2 or so years and I'm now on a road trip around the US. i've hauled up in vermont for the week to catch up on a bunch of stuff - top of the list is to try and get vfs up to scratch finally. i'm planning to be in the Massachusetts area next week - again for around a week. if you guys are avaiable, it'd be great to meet up and hash this out.

comment:9 Changed 6 years ago by cablehead

BTW - the decorator stuff is a general version of the stackable umask and sudo decorators from the crusty vfs stuff. If vfs is ever stable again, I think it'd be cool to bring back those decorators.

comment:10 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to cablehead
  • twisted/vfs/ivfs.py
    • we stopped using "api stability" markers - everything released is stable :) vfs is unreleased though.
    • we're also going to remove email addresses from maintainer fields.
    • I think we should come up with some adjustments for IVFSNode. It shouldn't need to replicate the abstract path representation API for FilePath (and particularly, each implementation of it shouldn't need to do this).
    • What does createDirectory do if there is already a file with the given name? If there is a directory? If there is a symlink?
    • createFile probably isn't expressive enough. This is related to #3123.
    • isdir/isfile/exists are redundant with getMetadata
    • What does rename do if the destination already exists and is a file? A symlink? A directory?
    • It might be useful if there were some way to determine which flags were applied by the implementation of open. Using os.O_* here seems fairly POSIX-centric. It's probably worth looking at SFTP v6 (<http://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-13.txt>) to see what they decided good cross-platform compromises were. Note that in SFTP, it's possible to query the filesystem to learn what flags it supports.
    • Oh uh. I see that open is, um, in place. That's no good. It needs to return a new object; that object should have the close, readChunk (but see below), and writeChunk (but see below).
    • readChunk and writeChunk might be useful utilities, but I'm relatively certain that the fundamental read/write API should be based on producers and consumers.
  • twisted/vfs/decorator.py
    • same comments about api stability. it may also be sensible to make this _decorator.py
  • twisted/vfs/test/test_decorator.py, twisted/vfs/test/test_backends.py
    • docstrings need reformatting
    • docstrings should also be more descriptive of what's being tested and what's expected
  • twisted/vfs/backends/init.py
    • if you can manage, it'd be good to get this file into a state where it's not "replaced". this can mess up svn sometimes (mostly with directories, but I'd rather be safe)
    • a lot of things are missing docstrings
    • some functions are named as though they wanted to be classes
    • these may need updating once the ivfs.py issues are settled

comment:11 Changed 4 years ago by exarkun

  • Resolution set to wontfix
  • Status changed from new to closed

VFS has been removed (#4934)

Note: See TracTickets for help on using tickets.