#1537 new defect

fix some Interface violations

Reported by: warner Owned by: daira
Priority: minor Milestone: soon
Component: code Version: 1.9.0a1
Keywords: interface cleanup Cc:
Launchpad Bug:

Description (last modified by daira)

Daira ran a script (in #1474) and found a list of places where classes that claim to implement some Interface don't in fact do it correctly. Here's the list and a quick note about what I think should be done about each:

  • foolscap.slicer.BaseUnslicer does not correctly implement foolscap.tokens.IUnslicer: The 'where' method was not provided.
    • ISTR BaseUnslicer.where is purely abstract, meant to be overridden by subclasses. Not sure if that makes it easier to remove the IUnslicer annotation from BaseUnslicer (and add it to all subclasses, which is kinda verbose), or to add a dummy method to BaseUnslicer (which throws NotImplementedError if ever called, and maybe a tiny test to keep that line of code exercised so the coverage metrics don't drop :)
  • foolscap.slicers.root.RootSlicer does not correctly implement foolscap.tokens.ISlicer: The implementation of slice violates its contract because implementation doesn't allow enough arguments.
    • I think (but not sure) RootSlicer.slice is never called.. it should probably be modified to match signatures with tokens.ISlicer and also throw an error if ever called, just to be sure
  • foolscap.referenceable.Referenceable does not correctly implement foolscap.ipb.IRemotelyCallable: The 'getInterfaceNames' method was not provided.
    • getInterfaceNames() looks like it's misspelled in ipb.py (should be singular, not plural)
  • foolscap.referenceable.RemoteReferenceOnly does not correctly implement foolscap.ipb.IRemoteReference: The 'callRemoteOnly' method was not provided. The 'callRemote' method was not provided.
    • IRemoteReference should be split, or moved: RemoteReferenceOnly isn't supposed to have callRemote/callRemoteOnly
  • allmydata.immutable.encode.Encoder does not correctly implement allmydata.interfaces.IEncoder: The 'set_params' method was not provided. The 'set_size' method was not provided.
    • IEncoder should probably be split into two interfaces (one for CRSEncoder with set_params/set_size, the rest for the higher-level encode.Encoder)
  • allmydata.immutable.offloaded.LocalCiphertextReader does not correctly implement allmydata.interfaces.IEncryptedUploadable: The 'set_upload_status' method was not provided.
    • should probably be implemented
  • allmydata.immutable.literal.LiteralProducer does not correctly implement twisted.internet.interfaces.IPushProducer: The 'pauseProducing' method was not provided.
    • should be implemented

Attachments (1)

fix-literal-pauseProducing.darcs.patch (22.6 KB) - added by davidsarah at 2011-10-03T19:55:11Z.
immutable/literal.py: add pauseProducing method to LiteralProducer?. refs #1537

Download all attachments as: .zip

Change History (13)

Changed at 2011-10-03T19:55:11Z by davidsarah

immutable/literal.py: add pauseProducing method to LiteralProducer?. refs #1537

comment:1 Changed at 2011-10-03T19:56:54Z by davidsarah

  • Keywords review-needed added
  • Milestone changed from undecided to 1.10.0

comment:2 Changed at 2011-10-03T21:29:40Z by zooko

  • Keywords reviewed added; review-needed removed

comment:3 Changed at 2011-10-07T19:39:47Z by david-sarah@…

In [5395/ticket999-S3-backend]:

immutable/literal.py: add pauseProducing method to LiteralProducer?. refs #1537

comment:4 Changed at 2011-10-10T19:53:52Z by david-sarah@…

In bc50edc86ef1c20b:

immutable/literal.py: add pauseProducing method to LiteralProducer?. refs #1537

comment:5 Changed at 2011-12-31T20:11:45Z by zooko

  • Keywords reviewed removed

comment:6 Changed at 2011-12-31T20:28:54Z by davidsarah

  • Keywords reviewed added
  • Owner changed from somebody to davidsarah
  • Status changed from new to assigned

The patch has not been applied to trunk, so it's correct to keep the 'reviewed' keyword until it is. I don't know what comment:4 was about.

comment:7 Changed at 2011-12-31T22:38:44Z by zooko

  • Description modified (diff)

comment:8 Changed at 2011-12-31T22:50:14Z by zooko

  • Keywords reviewed removed

I checked and the added pauseProducing method is indeed now in LiteralProducer. Trac annotate tells me that it was committed (in git world) by bc0d9b682e97c26d.

comment:9 Changed at 2012-03-12T19:28:09Z by davidsarah

  • Keywords interface cleanup added

comment:10 Changed at 2012-04-01T03:59:36Z by davidsarah

  • Milestone changed from 1.11.0 to 1.10.0

comment:11 Changed at 2012-12-20T16:42:33Z by davidsarah

  • Milestone changed from 1.10.0 to 1.11.0

comment:12 Changed at 2016-08-23T16:15:26Z by daira

  • Description modified (diff)
  • Owner changed from davidsarah to daira
  • Status changed from assigned to new
Note: See TracTickets for help on using tickets.