Merge lp:~flacoste/launchpad/bug-797917 into lp:launchpad

Proposed by Francis J. Lacoste
Status: Merged
Approved by: Francis J. Lacoste
Approved revision: no longer in the source branch.
Merged at revision: 13262
Proposed branch: lp:~flacoste/launchpad/bug-797917
Merge into: lp:launchpad
Diff against target: 0 lines
To merge this branch: bzr merge lp:~flacoste/launchpad/bug-797917
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+64852@code.launchpad.net

Commit message

[r=allenap][bug=797917] Map NoSuchDistroSeries (along with all other NotFoundError descendants to 404). Bonus points: get rid of DistroSeriesNotFound exception and convert all webservice_error calls to the newer error_status decorator. Plus convert all status codes to httplib constants!

Description of the change

= Summary =

This maps NotFound type exception to the 404 status code over the API instead
of the more generic 400 (BadRequest). I also includes a couple of other
exception clean-ups.

== Proposed fix ==

== Pre-implementation notes ==

* This is going to somewhat break backward compatibility as the exception
  raised by clients is to change. Both Robert and I thinks this isn't a big
  deal, since clients written to recover from these exceptions have to do it
  in a very fragile way (by looking at the error message).

== Implementation details ==

* Export all NotFoundError subclass to httplib.NOT_FOUND (404). I could remove
  a lot of rendundant webservice_error declaration.

I then search for similar exceptions that were also mapped to 400 and switched
them over to 404:

 * ComponentNotFound
 * PocketNotFound
 * NoSuchBranch
 * NoSuchSourcePackageName

As drive-bys, I removed the DistroSeriesNotFound exception defined in
archive.py and use the registry one: NoSuchDistroSeries.

I also substitute a couple of webservice_error to use the new class decorator
@error_status and use the symbolic constants defined in httplib instead of the
raw status code.

== Tests ==

./bin/test -vvt 'webservice/(xx-person.txt|xx-distribtion.txt|xx-packageset.txt)|archive.txt|archivepermission.txt'

== Demo and Q/A ==

>>> from launchpadlib.launchpad import Launchpad
>>> lp = Launchpad.login_with('bug-797917-qa', 'qastaging')
>>> lp.distributions['ubuntu'].getSeries('foobar')
HTTP 1.1 404 Not Found...

= Launchpad lint =

Checking for conflicts and issues in changed files.

I'Ll fix the lint post-review. (As well as probably fix replace all the other
webservice_error() instances in the source tree.)

Linting changed files:
  lib/lp/registry/errors.py
  lib/lp/soyuz/stories/webservice/xx-archive.txt
  lib/lp/code/errors.py
  lib/lp/soyuz/stories/webservice/xx-packageset.txt
  lib/lp/soyuz/doc/archive.txt
  lib/lp/soyuz/interfaces/archive.py
  lib/lp/app/errors.py
  lib/lp/soyuz/interfaces/packageset.py
  lib/lp/soyuz/interfaces/webservice.py
  lib/lp/registry/stories/webservice/xx-distribution.txt
  lib/lp/soyuz/model/archive.py
  lib/lp/soyuz/model/archivepermission.py
  lib/lp/registry/stories/webservice/xx-person.txt
  lib/lp/soyuz/doc/archivepermission.txt

./lib/lp/soyuz/stories/webservice/xx-archive.txt
      20: want exceeds 78 characters.
      87: want exceeds 78 characters.
     113: source exceeds 78 characters.
     120: narrative uses a moin header.
     131: source exceeds 78 characters.
     140: want exceeds 78 characters.
     145: source exceeds 78 characters.
     155: want exceeds 78 characters.
     160: source exceeds 78 characters.
     170: want exceeds 78 characters.
     175: source exceeds 78 characters.
     185: want exceeds 78 characters.
     189: narrative uses a moin header.
     272: want exceeds 78 characters.
     333: want exceeds 78 characters.
     341: narrative has trailing whitespace.
     346: source has trailing whitespace.
     374: source has trailing whitespace.
     464: want exceeds 78 characters.
     490: narrative uses a moin header.
     497: source exceeds 78 characters.
     506: source exceeds 78 characters.
     514: source exceeds 78 characters.
     520: narrative exceeds 78 characters.
     523: source exceeds 78 characters.
     530: narrative uses a moin header.
     558: narrative uses a moin header.
     587: narrative uses a moin header.
     766: narrative uses a moin header.
     768: narrative exceeds 78 characters.
     787: source exceeds 78 characters.
     797: narrative uses a moin header.
     824: want exceeds 78 characters.
     840: want exceeds 78 characters.
     854: narrative uses a moin header.
     943: narrative uses a moin header.
     956: narrative uses a moin header.
    1004: narrative uses a moin header.
./lib/lp/soyuz/stories/webservice/xx-packageset.txt
     348: source exceeds 78 characters.
     509: source exceeds 78 characters.
     575: want exceeds 78 characters.
     596: want exceeds 78 characters.
     605: want exceeds 78 characters.
     606: want exceeds 78 characters.
     625: want exceeds 78 characters.
     652: want exceeds 78 characters.
     653: want exceeds 78 characters.
     663: want exceeds 78 characters.
     664: want exceeds 78 characters.
     678: want exceeds 78 characters.
     797: want exceeds 78 characters.
     798: want exceeds 78 characters.
     819: want exceeds 78 characters.
     831: want exceeds 78 characters.
     832: want exceeds 78 characters.
     846: source exceeds 78 characters.
     855: source exceeds 78 characters.
     863: source exceeds 78 characters.
     871: source exceeds 78 characters.
     880: source exceeds 78 characters.
./lib/lp/app/errors.py
      72: E302 expected 2 blank lines, found 1
./lib/lp/soyuz/model/archive.py
    1537: local variable 'error' is assigned to but never used
./lib/lp/soyuz/model/archivepermission.py
     197: local variable 'e' is assigned to but never used
./lib/lp/soyuz/doc/archivepermission.txt
       1: narrative uses a moin header.
      73: source exceeds 78 characters.
     263: narrative uses a moin header.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Nice :)

review: Approve

Preview Diff

Empty