#4297 NORM Never A: read-activity: Uses deprecated http-over-IPv4 instead of Tubes

[Zarro Boogs per Child]

#4297: read-activity: Uses deprecated http-over-IPv4 instead of Tubes
----------------------------+-----------------------------------------------
  Reporter:  smcv           |       Owner:  tomeu             
      Type:  defect         |      Status:  new               
  Priority:  normal         |   Milestone:  Never Assigned    
 Component:  read-activity  |     Version:  Git as of bug date
Resolution:                 |    Keywords:  collaboration     
  Verified:  0              |  
----------------------------+-----------------------------------------------

Comment(by smcv):

 Since Read potentially transfers fairly large files, and it needs to do so
 in a unicast way, it would make sense for it to use stream tubes. We
 haven't quite finished implementing those in Salut yet (#4241), but it'd
 make sense for Read to be the first activity to use them. However, just
 for the record, I'm going to describe how I'd implement Read if we still
 only had D-Bus tubes.

 ----

 I don't think the Read D-Bus API Erik implemented is ideal by any means.
 At some point I'm going to go round the core activities critiquing their
 D-Bus APIs and suggesting how to make them better, so the hypothetical API
 Erik wrote gets to be the first victim...

 Coding style: Far from PEP-8. See http://www.python.org/dev/peps/pep-0008/

 It makes no sense to try to transfer the document from someone who doesn't
 have all of it yet. The initiator of the activity is the only one who is
 guaranteed to have the document, so if he's still around, everyone else
 should probably try to get the document from him. If he's left, those
 without the document should fall back to trying to fetch it from a random
 participant, retrying with a different participant on error.

 File transfer should be done in chunks, that's correct. However, it makes
 no sense to emit the file data in broadcast signals that go to everyone,
 with most recipients ignoring them!

 dbus.connection.Connection.remove_signal_receiver() doesn't work like you
 think it does; dbus-python is not pygobject. Please read the
 documentation. add_signal_receiver() doesn't return an ID like in
 libgobject, it returns an SignalMatch object which has a remove() method.
 The SignalMatch object can't be passed to remove_signal_receiver (although
 perhaps it should be accepted).

 The hashing-buddy-keys stuff is just weird. Just use the Buddy's D-Bus
 object path if you need a unique identifier (I can't remember whether it's
 buddy.object_path, buddy.object_path() or buddy.get_object_path() right
 now).
 The D-Bus API I'd suggest (assuming only files of <4GB size need to be
 supported) would be that everyone who has a complete file (and only those
 people) exports an object onto the tube (call the class ReadableFile or
 something), with an interface with the following methods:

     method `GetSize () -> u: size`

     Return the size of the file (so you can populate the progress bar).

     method `ReadChunk (u: offset, q: count) -> ay: bytes`

     Read ''count'' bytes from the file starting at offset ''offset'' (0 =
 beginning of file). Return those bytes.

     The returned byte-array will have exactly ''count'' bytes unless end-
 of-file is reached, in which case fewer bytes (possibly 0) will be
 returned.

 (u is dbus.UInt32, q is dbus.UInt16, ay is dbus.ByteArray.Refer to
 http://dbus.freedesktop.org/doc/dbus-python/doc/tutorial.html#return-
 values-and-the-byte-arrays-and-utf8-strings-options for efficient byte-
 array handling.)

 Then recipients can just call ReadChunk, repeatedly, saving the resulting
 bytes to the local file. Recipients don't need to export an object until
 they have the whole file, and indeed should not do so. When they have the
 whole file, they should create a ReadableFile from it and export that onto
 the tube for others to download.

 When recipients try to fetch the file from someone who doesn't have it,
 they'll get a DBusException from either of those methods (either because
 there is no ReadableFile object at the desired object path, or because
 GetSize/ReadChunk explicitly raises an exception), which you can catch and
 use to retry.

-- 
Ticket URL: <https://dev.laptop.org/ticket/4297#comment:3>
One Laptop Per Child <https://dev.laptop.org>
OLPC bug tracking system