Merge lp://staging/~matsubara/launchpad/bug-606184-pageid-for-collections into lp://staging/launchpad

Proposed by Diogo Matsubara
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 11467
Proposed branch: lp://staging/~matsubara/launchpad/bug-606184-pageid-for-collections
Merge into: lp://staging/launchpad
Diff against target: 93 lines (+50/-2)
2 files modified
lib/canonical/launchpad/webapp/servers.py (+18/-1)
lib/canonical/launchpad/webapp/tests/test_pageid.py (+32/-1)
To merge this branch: bzr merge lp://staging/~matsubara/launchpad/bug-606184-pageid-for-collections
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+33801@code.staging.launchpad.net

Commit message

Fixes bug 606184 by including the origin page resource in the page id for ICollectionResource views.

Description of the change

== Summary ==

This branch fixes bug 606184 by including the origin page resource in the page
id for ICollectionResource views.

== Proposed fix ==

When the page id is generated for API requests, the code inspects if the view
is a ICollectionResource, then inspects the type_url attribute and add part of
it as the collection identifier to the page id.

== Pre-implementation notes ==

I had a pre-imp with Gary. Initially Gary suggested that this could be
accomplished with adapters and I tried that route but got stuck. Then we
agreed to change the implementation to be simpler so I could move this forward.

== Implementation details ==

The origin page resource comes from ICollectionResource.type_url, so I used
that as the identifier for the given collection and added that as part of the
page id. That way when engineers are analysing such OOPS summaries they'll be
able to easily see if the OOPS is in their domain or not, as requested in
the bug report.

== Tests ==

bin/test -tt test_pageid

== Demo and Q/A ==

Once the code lands on staging, it's enough to check a staging oops summary
and see if the "scopedcollection:collectionresource" oopses contain the
additional information about the origin page resource.

== Lint ==

No lint left over in the two files modified by this branch.

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

merge-conditional

Cool, thank you.

My only comment is that I'd like to see more of a comment explaining what is going on. things to explain: what is a collection_identifier? How do you use it to relate a page id to actual code, if you are trying to debug an OOPS? Extra credit: what code is responsible for generating the collection_identifier?

Please answer at least the first two questions, and also add the information to the page ids section of https://dev.launchpad.net/Foundations/Webservice

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.