launchpad-dev team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Hi folks,

A Request-For-Answers here. I'm currently working on fixing bug 657004
("can't read bug.linked_branches via REST API when it contains a
private branch"). The most efficient solution I've been able to come
up with (with lots of help and advice from Rob) adds a getter,
getVisibleLinkedBranches(user) to IBug, which returns all the linked
branches visible to `user`. This is now exposed in the API (in my
branch [1]) as IBug.linked_branches, replacing the existing collection
attribute.

I didn't expect this to work, but with the following declaration:

    @call_with(user=REQUEST_USER)
    @export_operation_as('linked_branches')
    @export_read_operation()
    @operation_for_version('devel')
    def getVisibleLinkedBranches(user):
        """Rertun the linked to this bug that are visible by `user`."""

The following launchpadlib call works perfectly:

    >>> linked_branches = lp.bug[12345].linked_branches

(I'd expected that I'd need to add parens after the call because it's
become a named get instead of a property on the bug. Launchpadlib
appears to do magic, which is slightly worrying).

The problem that I now have is that although for launchpadlib users
the API won't appear to have changed, at the RESTful level it *has*
changed:

 - There's no longer a linked_branches_collection_link in
   the Bug JSON.
 - To get linked_branches you now have to make a named GET
   to linked_branches.

Basically, if I'm right, any API client that uses the RESTful API
directly - including our own JS API client - is now going to break
when it tries to do anything with linked_branches.

I can see a couple of solutions to this:

 1. Make linked_branches a property again. This means using
    the LaunchBag to get the current user. Avoiding the Bag
    was the main reason for adding a getter instead of using
    a property here.
 2. Leave the current linked_branches attribute exposed for
    the stable versions of the API and expose the new getter
    for the devel API (and subsequent stable versions).
 3. Change the API and contact all the people who use the WS
    API to let them know about the change.

Thoughts:

 - Option 1 seems like the simplest, though I dislike using
   the LaunchBag.
 - Option 2 seems like a half-decent option, though I don't
   know if it's possible to expose an attribute for only a
   given set of API versions (as one would an operation with
   @operation_for_version). Also, this then means that the bug
   itself hasn't been fixed except for the devel version of the
   API.
 - Option 3 is the most complete solution in terms of not using
   the LaunchBag and using a getter which receives a user
   parameter. However, it explicitly violates our
   backwards-compatibility rules regarding the WS API and means
   that we're probably going to end up causing some pain for API
   users, regardless of how much we try to mitigate that pain.

At the moment I'm leaning towards option #1, but I'd appreciate
suggestions, ideas or instructions from the peanut gallery at this
point.

Yours in vague hope,

Graham

 [1] https://code.launchpad.net/~gmb/launchpad/private-branches-bug-657004/+merge/65639

-- 
Graham Binns | PGP Key: EC66FA7D
http://launchpad.net/~gmb