j.c.sackett has proposed merging
lp:~jcsackett/launchpad/urlparse-does-not-make-me-happy into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #823471 in Launchpad itself: "privacy ribbon does not appear on secondary
sites"
https://bugs.launchpad.net/launchpad/+bug/823471
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/urlparse-does-not-make-me-happy/+merge/82415
Summary
=======
A previous branch was landed to allow our launchpad_loggerhead tools to ping
the API to determine if a privacy notification should be shown on loggerhead.
That branch used urlparse.urljoin to construct the API url, which resulted in
the wrong url being created, as demonstrated by William Grant.
This branch does a bit of cleanup and adds some robustness to the privacy code,
as well as addressing the bad url construction.
Preimp
======
Spoke with William Grant, Curtis Hovey
Implementation
==============
Instead of using urlparse.urljoin, this branch just manually builds the url
from the components.
It also adds 401 to the list of return codes that implies a private branch, and
closes the response once it's done checking the status.
QA
==
Go to a private branch; when opening it in loggerhead, the privacy ribbon
should display. When opening a pubic branch in loggerhead, it should not.
Lint
====
This branch is lint free.
--
https://code.launchpad.net/~jcsackett/launchpad/urlparse-does-not-make-me-happy/+merge/82415
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jcsackett/launchpad/urlparse-does-not-make-me-happy into lp:launchpad.
=== modified file 'lib/launchpad_loggerhead/app.py'
--- lib/launchpad_loggerhead/app.py 2011-10-26 16:21:36 +0000
+++ lib/launchpad_loggerhead/app.py 2011-11-16 15:47:33 +0000
@@ -238,8 +238,12 @@
if not os.path.isdir(cachepath):
os.makedirs(cachepath)
self.log.info('branch_url: %s', branch_url)
- branch_api_url = urlparse.urljoin(
- config.appserver_root_url('api'), 'devel', branch_name)
+ base_api_url = config.appserver_root_url('api')
+ branch_api_url = '%s/%s/%s' % (
+ base_api_url,
+ 'devel',
+ branch_name,
+ )
self.log.info('branch_api_url: %s', branch_api_url)
req = urllib2.Request(branch_api_url)
private = False
@@ -247,16 +251,19 @@
# We need to determine if the branch is private
response = urllib2.urlopen(req)
except urllib2.HTTPError as response:
- if response.getcode() in (400, 403):
- ## 400 and 403 are the possible returns for api requests
- ## on a private branch without authentication.
+ 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.
self.log.info("Branch is private")
private = True
self.log.info(
"Branch state not determined; api error, return code: %s",
- response.getcode())
+ code)
+ response.close()
else:
self.log.info("Branch is public")
+ response.close()
try:
bzr_branch = safe_open(
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
