Merge lp:~bac/lp2kanban/kill-httplib2 into lp:~launchpad/lp2kanban/trunk

[Brad Crittenden]

On the new AMI we're forced to use in canonistack, the cert for leankitkanban cannot be verified because httplib2 keeps its own copy of cacerts.txt and it does not appear to have the CA for the cert. Note the cert is valid as modern browsers verify and accept it.

Switching from httplib2 with that deficiency to the new hotness 'requests' allows cert verification. Probably a good move for the long run anyway.

[Benji York (benji)]

Looks great.

review: Approve (code)

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

[Björn Tillenius (bjornt)]

Does 'requests' support using a proxy? With httplib2 you can set the https_proxy environment variable, but I can't get it to work with this branch.

review: Needs Information

[Brad Crittenden (bac)]

Bjorn it does support proxies but perhaps not through the same mechanism. If that is important to you I'll defer landing this branch until it is resolved. Thanks for bringing it up.

[Björn Tillenius (bjornt)]

On Fri, May 10, 2013 at 10:26:24PM -0000, Brad Crittenden wrote:
> Bjorn it does support proxies but perhaps not through the same
> mechanism. If that is important to you I'll defer landing this branch
> until it is resolved. Thanks for bringing it up.

Thanks. Yes, we do depend on proxy support, since we run the script on
an IS computer, which doesn't allow direct outgoing connections.

[Brad Crittenden (bac)]

Bjorn 'requests' claims to support the HTTPS_PROXY environment variable. I haven't set up an environment to test it yet.

Could you review their docs and perhaps test again to verify it really doesn't work?

http://docs.python-requests.org/en/latest/user/advanced/?highlight=https_proxy#proxies

[Björn Tillenius (bjornt)]

On Tue, May 14, 2013 at 04:05:30PM -0000, Brad Crittenden wrote:
> Bjorn 'requests' claims to support the HTTPS_PROXY environment
> variable. I haven't set up an environment to test it yet.
>
> Could you review their docs and perhaps test again to verify it really
> doesn't work?

Indeed, I can see that there's some support for it. I'm debugging now
why doesn't work...

--
Björn Tillenius | https://launchpad.net/~bjornt

[Björn Tillenius (bjornt)]

On Tue, May 14, 2013 at 04:05:30PM -0000, Brad Crittenden wrote:
> Bjorn 'requests' claims to support the HTTPS_PROXY environment
> variable. I haven't set up an environment to test it yet.
>
> Could you review their docs and perhaps test again to verify it really
> doesn't work?

So, if I change the code to use self.http.request() instead of creating
the Request manually, it does try to use the proxy. However, it still
doesn't work, I get a 501 error, method not allowed. I suspect that it
tries to use CONNECT, instead of GET. I don't see a way of changing the
way it uses the proxy, but I might be missing something.

BTW, I would be fine with no proxy support, if we could get Leankit to
provide a stable IP for the API endpoint. At the moment it's changing
all the time, which means that we can't simply open up the firewall to
allow that traffic.

--
Björn Tillenius | https://launchpad.net/~bjornt

[lp:~bac/lp2kanban/kill-httplib2]

87. By Brad Crittenden on 2013-05-16

Use an older version of requests to get around https proxy bug.

[Brad Crittenden (bac)]

Hi Bjorn,

I found this discussion about https proxy and requests:
https://github.com/kennethreitz/requests/issues/905

In that thread there was mention that 0.7.6 of requests was the last version that appeared to work. I've just pushed an lp2kanban branch that specifies that version. Could you test it in your environment?

[lp:~bac/lp2kanban/kill-httplib2]

88. By Brad Crittenden on 2013-05-16

requests API changes

89. By Brad Crittenden on 2013-05-16

Changes to revert back to request 0.7.6

[Björn Tillenius (bjornt)]

Thanks Brad, 0.7.6 does work. You need to apply the following patch to your branch, though, otherwise the proxy won't be used:

  http://pastebin.ubuntu.com/5673322/

And while you're at it, maybe you could remove the AttributeError handler in the except clause, since it's httplib2-specific?

review: Approve

[lp:~bac/lp2kanban/kill-httplib2]

90. By Brad Crittenden on 2013-05-17

Fix for https proxy as suggested by Bjorn in review.

[Brad Crittenden (bac)]

Thanks for the QA and patch Bjorn. Finally landed.

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-11-23 13:22:23 +0000
+++ buildout.cfg	2013-05-16 15:17:25 +0000
@@ -29,7 +29,9 @@
 [versions]
 argparse = 1.2
 elementtree = 1.2.7-20070827-preview
-httplib2 >= 0.7.0
+# The last version of requests to use urllib2 instead of urllib3 is 0.7.6.
+# This avoids an urllib3 bug that causes https_proxy to not work.
+requests = 0.7.6
 keyring = 0.5.1
 launchpadlib = 1.9.8
 lazr.authentication = 0.1.2

=== modified file 'setup.py'
--- setup.py	2013-01-18 17:45:01 +0000
+++ setup.py	2013-05-16 15:17:25 +0000
@@ -15,7 +15,8 @@
         'argparse',
         'launchpadlib',
         'lazr.uri',
+        'requests',
+        'setuptools',
         'testtools',
-        'setuptools',
         ],
     )

=== modified file 'src/lp2kanban/bugs2cards.py'
--- src/lp2kanban/bugs2cards.py	2013-04-08 12:46:05 +0000
+++ src/lp2kanban/bugs2cards.py	2013-05-16 15:17:25 +0000
@@ -2,7 +2,6 @@
 #
 from ConfigParser import ConfigParser
 from argparse import ArgumentParser
-import httplib2
 from launchpadlib.launchpad import Launchpad
 from lp2kanban.cards2workitems import (
     convert_cards_to_work_items,
@@ -424,7 +423,7 @@
     else:
         lp = Launchpad.login_anonymously(
             'lp2kanban', config['lp_server'], version="devel",
-            launchapdlib_dir=launchpadlib_dir)
+            launchpadlib_dir=launchpadlib_dir)
         print ("   Accessing Launchpad anonymously.  "
                "Private bugs will not be synced.")
     return lp
@@ -578,7 +577,8 @@
     args = get_arg_parser().parse_args()
 
     if args.debug:
-        httplib2.debuglevel = 1
+        import httplib
+        httplib.HTTPConnection.debuglevel = 1
 
     config = ConfigParser()
     config.read(args.config)

=== modified file 'src/lp2kanban/kanban.py'
--- src/lp2kanban/kanban.py	2013-04-16 16:57:45 +0000
+++ src/lp2kanban/kanban.py	2013-05-16 15:17:25 +0000
@@ -1,7 +1,7 @@
 # Copyright 2011-2012 Canonical Ltd
 # -*- coding: utf-8 -*-
 
-import httplib2
+import requests
 import json
 import operator
 from pprint import pprint
@@ -63,9 +63,9 @@
 
     def _configure_auth(self, username=None, password=None):
         """Configure the http object to use basic auth headers."""
-        http = httplib2.Http()
+        http = requests.sessions.Session()
         if username is not None and password is not None:
-            http.add_credentials(username, password)
+            http.auth = (username, password)
         return http
 
     def post(self, url, data, handle_errors=True):
@@ -89,8 +89,14 @@
         self.last_request_time = time.time()
         try:
             #print "Fetching " + self.base_api_url + url + "..."
-            resp, content = self.http.request(
-                self.base_api_url + url, action, data, headers)
+            request = requests.Request(
+                method=action,
+                url=self.base_api_url + url,
+                data=data,
+                auth=self.http.auth,
+                headers=headers,
+                config={})
+            sent = request.send()
         except AttributeError as e:
             # Weirdly, httplib2 has a habit of throwing an AttributeError
             # when it can't connect, so we handle that nicely.
@@ -98,12 +104,12 @@
         except Exception as e:
             raise IOError("Unable to make HTTP request: %s" % e.message)
 
-        if int(resp['status']) not in LeankitResponseCodes.SUCCESS_CODES:
+        if not sent or request.response.status_code not in LeankitResponseCodes.SUCCESS_CODES:
             print "Error from kanban"
-            pprint(resp)
-            raise IOError('kanban error %s' % (resp['status']))
+            pprint(request.response)
+            raise IOError('kanban error %d' % (request.response.status_code))
 
-        response = Record(json.loads(content))
+        response = Record(json.loads(request.response.content))
 
         if (handle_errors and
             response.ReplyCode not in LeankitResponseCodes.SUCCESS_CODES):