Merge lp://staging/~cody-somerville/offspring/fix-offspring-on-python2.7 into lp://staging/offspring

Proposed by Cody A.W. Somerville
Status: Merged
Approved by: Timothy R. Chavez
Approved revision: 156
Merged at revision: 152
Proposed branch: lp://staging/~cody-somerville/offspring/fix-offspring-on-python2.7
Merge into: lp://staging/offspring
Diff against target: 153 lines (+74/-8)
5 files modified
Makefile (+1/-1)
config/offspring-web.wsgi (+4/-1)
docs/install.rst (+3/-2)
lib/offspring/networking.py (+11/-4)
lib/offspring/tests/test_networking.py (+55/-0)
To merge this branch: bzr merge lp://staging/~cody-somerville/offspring/fix-offspring-on-python2.7
Reviewer Review Type Date Requested Status
Timothy R. Chavez Approve
Review via email: mp+116500@code.staging.launchpad.net

Description of the change

Removes need to run Offspring on python2.6 by fixing compatibility issues with >python2.7 xmlrpclib API.

Still need to do further smoke testing of codebase running python2.7 but wanted to get this initial work up for review. Additional suggestions for improved tests appreciated.

To post a comment you must log in.
Revision history for this message
Timothy R. Chavez (timrchavez) wrote :

51 + connection = TimeoutHTTPConnection(host)
52 + self._connection = host, connection

Why does self._connection get set for sys.version_info >= 2.7 but not < 2.7?

Revision history for this message
Cody A.W. Somerville (cody-somerville) wrote :

Python2.7 adds support to xmlrpclib for HTTP/1.1 - specifically gzip encoding and keep-alive. To support the latter, make_connection needs to return the same connection if one is available. Our subclass does not do this (so does not support keep-alive) but the 'close' method added to the parent class expects it to be present so added that line pro-actively to prevent it from throwing an exception.

The code we'd need to add to the top of our make_connection method to support keep-alive (and not break python2.6) is as follows in case we ever want to add it:

        # Return an existing connection if possible for HTTP/1.1 keep-alive
        if hasattr(self, '_connection') and host == self._connection[0]:
            return self._connection[1]

Revision history for this message
Timothy R. Chavez (timrchavez) wrote :

<timrc> Cody, maybe we should add the self._connection line commented out with a note since it's not actually doing anything right now (or that's how I understood your comment)
<timrc> Cody, also do we want to remove the hardcoded python2.6 paths from at least bugsy/config/offspring-web.wsgi (and maybe bugsy/config/offspring-web.apache)? And along with that, do we need to update docs/install.rst at all?

Revision history for this message
Timothy R. Chavez (timrchavez) wrote :

OK, sorry, re-read and understand that it's expected to be there. Perhaps a comment? I would ask we take out the explicit mention of python2.6 from docs/installing.rst (or mention alternative packages for python2.7) and make a note that paths should be updated to python2.7 from python2.6 in the wsgi and apache files.

review: Approve
153. By Cody A.W. Somerville

Added comment about self._connection per request in review.

154. By Cody A.W. Somerville

Updated wsgi file to use path to site-packages in virtualenv based on version of python running.

155. By Cody A.W. Somerville

Updated installation documentation: removed python2.6 and python2.6-dev from package dependencies and added note to ensure alias in apache2 configuration file for django's media files points are correct path for the version of python being used.

156. By Cody A.W. Somerville

Fixed storing of extra headers - needs to be stored to self._extra_headers and not self.extra_headers.

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.

Subscribers

People subscribed via source and target branches