[tahoe-lafs-trac-stream] [tahoe-lafs] #1429: automatically upload a file when it is put in a given local directory

[tahoe-lafs]

#1429: automatically upload a file when it is put in a given local directory
-------------------------+-------------------------------------------------
     Reporter:           |      Owner:
  davidsarah             |     Status:  new
         Type:  defect   |  Milestone:  soon
     Priority:  major    |    Version:  1.8.2
    Component:  code-    |   Keywords:  drop-upload inotify usability
  frontend               |  design-review-needed
   Resolution:           |
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Comment (by davidsarah):

 Replying to [comment:6 zooko]:
 > Here is a review of attachment:drop-upload.darcs.patch .
 >
 > First of all: yay! This feature is neat. :-)

 Thanks :-)

 >  * What is the "FIXME: Unicode paths"? I don't see what needs to change
 there to handle non-ASCII paths.

 {{{tahoe.cfg}}} is encoded in UTF-8, but the path wants to be in
 {{{sys.getfilesystemencoding()}}}.

 >  * Why are we catching {{{Exception}}}--was it just because we might
 have Twisted < 10.1? We should remove any checks for if we have Twisted <
 10.1 (except, of course, for the checks we do for all dependencies in
 [source:src/allmydata/__init__.py]).

 If there is any exception here, the node will silently fail to start. (The
 same is true of exceptions during initialization of the other frontends.)
 We should fix that in a better way -- it seems relevant to #355 and #1360
 for example -- but I wanted to avoid this failure mode while I was
 debugging.

 >  * "FIXME: error reporting" sounds like a serious incomplete part, but
 what exactly should be added? Logging to the Tahoe-LAFS logging system of
 certain sorts of failures? What could fail here?

 We should raise an exception with a proper message (and ensure that gets
 logged) rather than failing an assert.

 >  * Perhaps the logging in _notify() is too verbose.
 >  * the "dump event to file" part of _notify() is just for debugging and
 should be removed.

 Yes, it was just easier to see the events that way while prototyping.
 Logging to the Twisted log is sufficient, and the message could be
 shorter.

 >  * "FIXME: what happens if we get another event for the same file while
 it is uploading?" Well, I looked at the code of {{{dirnode.add_file()}}}
 and {{{Uploader.upload()}}} and I guess it just redundantly uploads it a
 second time.

 Yes, I think this is fairly harmless, modulo the fact that we should
 probably be using a more restrictive event mask.

 > (See also #952, which is about what happens when you upload the
 bytewise-identical file more than once at a time through the WAPI.)  What
 ''should'' it do? Well, what should the user experience look like? You
 drag and drop a file into this directory, and then you drag and drop a new
 version of it one second later, and then what? Maybe the visual display of
 the directory should have been updated as soon as you dropped the first
 one to show that it is in the middle of uploading that file. Then, if you
 drop another version on it, the display should update to show that there
 is another update scheduled to happen after this one completes. So I guess
 we need a new ticket for a GUI frontend to this functionality (integrated
 into some extant GUI like Nautilus, I suppose). But at this layer, I guess
 what it should do is schedule another upload for as soon as this upload is
 finished ''unless'' another such upload is already scheduled, in which
 case it should do nothing.

 Right, I'll open another ticket about suppressing redundant uploads.

 >  * "FIXME: should all notified events cause an upload?": from
 http://www.kernel.org/doc/man-pages/online/pages/man7/inotify.7.html and
 http://inotify.aiken.cz/?section=inotify&page=faq&lang=en it looks like we
 should respond to {{{IN_CLOSE_WRITE | IN_CREATE | IN_MOVE_SELF |
 IN_MOVE_TO}}}

 That looks about right, although IN_MOVE_SELF should be treated
 differently, since it's moving the local directory away from its current
 path. I don't know what that should do -- probably the safest thing is to
 disable the drop-upload feature entirely until the node is restarted (and
 say "don't do that" in the docs).

 >  * Also of course the other FIXMEs deserve to be investigated more. :-)
 >
 > Other than that, it looks good. It is neat how few lines of code are
 needed to add this functionality! :-)

 Yeah, {{{twisted.internet.inotify}}} rocks!

-- 
Ticket URL: <http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1429#comment:7>
tahoe-lafs <http://tahoe-lafs.org>
secure decentralized storage