launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/404s-also-private into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #897296 in Launchpad itself: "Privacy ribbon doesn't show up for private branches on private teams on codebrowse"
  https://bugs.launchpad.net/launchpad/+bug/897296

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/404s-also-private/+merge/83639

Summary
=======
In lib/launchpad_loggerhead/app.py, we check if the return code from an anon request to the api for a branch to see if it's private. 404 was not one of the return codes checked for, but it is returned for private branches on private teams, which we take a policy of saying do not exist. This adds 404 to the list of private-related return codes.

Implementation
==============
There's a check in lib/launchpad_loggerhead/app.py to see if the return code is in a tuple of expected codes. 404 has been added to that tuple. Comments have also been updated to explain why those codes are there.

Tests
=====
Unfortunately, we have no real mechanism for testing the integration of loggerhead and launchpad.

QA
==
Check a private branch on a private team in loggerhead; the privacy ribbon should show up.

Lint
====
This branch is lint free.

-- 
https://code.launchpad.net/~jcsackett/launchpad/404s-also-private/+merge/83639
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/404s-also-private into lp:launchpad.

=== modified file 'lib/launchpad_loggerhead/app.py'
--- lib/launchpad_loggerhead/app.py	2011-11-16 15:38:18 +0000
+++ lib/launchpad_loggerhead/app.py	2011-11-28 16:50:39 +0000
@@ -252,9 +252,16 @@
                 response = urllib2.urlopen(req)
             except urllib2.HTTPError as response:
                 code = response.getcode()
-                if code in (400, 401, 403):
-                    # 400, 401, and 403 are the possible returns for api
-                    # requests on a private branch without authentication.
+                if code in (400, 401, 403, 404):
+                    # There are several error codes that imply private data.
+                    # 400 (bad request) is a default error code from the API
+                    # 401 (unauthorized) should never be returned as the
+                    # requests are always from anon. If it is returned
+                    # however, the data is certainly private.
+                    # 403 (forbidden) is obviously private.
+                    # 404 (not found) implies privacy from a private team or
+                    # similar situation, which we hide as not existing rather
+                    # than mark as forbidden.
                     self.log.info("Branch is private")
                     private = True
                 self.log.info(