Merge lp:~gz/pyjuju/old_twisted_1029070 into lp:pyjuju

Proposed by Martin Packman
Status: Merged
Approved by: Jim Baker
Approved revision: 575
Merged at revision: 575
Proposed branch: lp:~gz/pyjuju/old_twisted_1029070
Merge into: lp:pyjuju
Diff against target: 105 lines (+16/-13)
3 files modified
juju/providers/openstack/client.py (+10/-7)
juju/providers/openstack/provider.py (+5/-4)
juju/providers/openstack/tests/test_client.py (+1/-2)
To merge this branch: bzr merge lp:~gz/pyjuju/old_twisted_1029070
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+122849@code.launchpad.net

Description of the change

Tolerate older twisted versions in openstack provider

The juju ppa includes newer versions of txaws and txzookeeper, but uses
the system twisted version. This means it ftbfs on natty and oneiric,
which doesn't matter much as most devs should be using something newer
anyway, but is not hard to fix. This branch uses some existing compat
code and adds a conditional import, which is enough to make happiness.

https://codereview.appspot.com/6496084/

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :
Download full text (5.3 KiB)

Reviewers: mp+122849_code.launchpad.net,

Message:
Please take a look.

Description:
Tolerate older twisted versions in openstack provider

The juju ppa includes newer versions of txaws and txzookeeper, but uses
the system twisted version. This means it ftbfs on natty and oneiric,
which doesn't matter much as most devs should be using something newer
anyway, but is not hard to fix. This branch uses some existing compat
code and adds a conditional import, which is enough to make happiness.

https://code.launchpad.net/~gz/juju/old_twisted_1029070/+merge/122849

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6496084/

Affected files:
   A [revision details]
   M juju/providers/openstack/client.py
   M juju/providers/openstack/provider.py
   M juju/providers/openstack/tests/test_client.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: juju/providers/openstack/client.py
=== modified file 'juju/providers/openstack/client.py'
--- juju/providers/openstack/client.py 2012-09-03 11:08:13 +0000
+++ juju/providers/openstack/client.py 2012-09-05 11:10:08 +0000
@@ -30,10 +30,13 @@
  from twisted.internet.interfaces import IProducer
  from twisted.internet import reactor

-from twisted.web import (
- client,
- http_headers,
- )
+from twisted.web.client import Agent
+# Older twisted versions don't expose _newclient exceptions via client
module
+try:
+ from twisted.web.client import ResponseFailed
+except ImportError:
+ from twisted.web._newclient import ResponseFailed
+from twisted.web.http_headers import Headers
  from zope.interface import implements

  from juju import errors
@@ -100,7 +103,7 @@
  def _translate_response_failed(failure):
      """Turn internal twisted client failures into juju exceptions"""
      txerr = failure.value
- if isinstance(txerr, client.ResponseFailed):
+ if isinstance(txerr, ResponseFailed):
          for reason in txerr.reasons:
              err = reason.value
              if isinstance(err, SSLError):
@@ -110,7 +113,7 @@

  @inlineCallbacks
  def request(method, url, extra_headers=(), body=None, check_certs=False):
- headers = http_headers.Headers({
+ headers = Headers({
          # GZ 2012-07-03: Previously passed Accept: application/json header
          # here, but not always the right thing. Bad for
swift?
          "User-Agent": [_USER_AGENT],
@@ -128,7 +131,7 @@
      kwargs = {}
      if check_certs:
          kwargs['contextFactory'] = WebVerifyingContextFactory()
- agent = client.Agent(reactor, **kwargs)
+ agent = Agent(reactor, **kwargs)
      response = yield agent.request(method, url, headers, body).addErrback(
          _translate_response_failed)
      if response.length == 0:

Index: juju/providers/openstack/provider.py
=== modified file 'juju/providers/openstack/provider.py'
--- juju/providers/openstack/provider.py ...

Read more...

Revision history for this message
Jim Baker (jimbaker) wrote :

+1, this compatibility fix LGTM

https://codereview.appspot.com/6496084/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'juju/providers/openstack/client.py'
--- juju/providers/openstack/client.py 2012-09-03 11:08:13 +0000
+++ juju/providers/openstack/client.py 2012-09-05 11:29:20 +0000
@@ -30,10 +30,13 @@
30from twisted.internet.interfaces import IProducer30from twisted.internet.interfaces import IProducer
31from twisted.internet import reactor31from twisted.internet import reactor
3232
33from twisted.web import (33from twisted.web.client import Agent
34 client,34# Older twisted versions don't expose _newclient exceptions via client module
35 http_headers,35try:
36 )36 from twisted.web.client import ResponseFailed
37except ImportError:
38 from twisted.web._newclient import ResponseFailed
39from twisted.web.http_headers import Headers
37from zope.interface import implements40from zope.interface import implements
3841
39from juju import errors42from juju import errors
@@ -100,7 +103,7 @@
100def _translate_response_failed(failure):103def _translate_response_failed(failure):
101 """Turn internal twisted client failures into juju exceptions"""104 """Turn internal twisted client failures into juju exceptions"""
102 txerr = failure.value105 txerr = failure.value
103 if isinstance(txerr, client.ResponseFailed):106 if isinstance(txerr, ResponseFailed):
104 for reason in txerr.reasons:107 for reason in txerr.reasons:
105 err = reason.value108 err = reason.value
106 if isinstance(err, SSLError):109 if isinstance(err, SSLError):
@@ -110,7 +113,7 @@
110113
111@inlineCallbacks114@inlineCallbacks
112def request(method, url, extra_headers=(), body=None, check_certs=False):115def request(method, url, extra_headers=(), body=None, check_certs=False):
113 headers = http_headers.Headers({116 headers = Headers({
114 # GZ 2012-07-03: Previously passed Accept: application/json header117 # GZ 2012-07-03: Previously passed Accept: application/json header
115 # here, but not always the right thing. Bad for swift?118 # here, but not always the right thing. Bad for swift?
116 "User-Agent": [_USER_AGENT],119 "User-Agent": [_USER_AGENT],
@@ -128,7 +131,7 @@
128 kwargs = {}131 kwargs = {}
129 if check_certs:132 if check_certs:
130 kwargs['contextFactory'] = WebVerifyingContextFactory()133 kwargs['contextFactory'] = WebVerifyingContextFactory()
131 agent = client.Agent(reactor, **kwargs)134 agent = Agent(reactor, **kwargs)
132 response = yield agent.request(method, url, headers, body).addErrback(135 response = yield agent.request(method, url, headers, body).addErrback(
133 _translate_response_failed)136 _translate_response_failed)
134 if response.length == 0:137 if response.length == 0:
135138
=== modified file 'juju/providers/openstack/provider.py'
--- juju/providers/openstack/provider.py 2012-09-01 14:46:03 +0000
+++ juju/providers/openstack/provider.py 2012-09-05 11:29:20 +0000
@@ -12,9 +12,10 @@
1212
13import logging13import logging
1414
15from twisted.internet.defer import inlineCallbacks, returnValue, gatherResults15from twisted.internet.defer import inlineCallbacks, returnValue
1616
17from juju import errors17from juju import errors
18from juju.lib.twistutils import gather_results
18from juju.lib.cache import CachedValue19from juju.lib.cache import CachedValue
19from juju.providers.common.base import MachineProviderBase20from juju.providers.common.base import MachineProviderBase
2021
@@ -181,7 +182,7 @@
181 :rtype: :class:`twisted.internet.defer.Deferred`182 :rtype: :class:`twisted.internet.defer.Deferred`
182 """183 """
183 machines = yield self.get_machines()184 machines = yield self.get_machines()
184 deleted_machines = yield gatherResults(185 deleted_machines = yield gather_results(
185 [self._delete_machine(m, True) for m in machines])186 [self._delete_machine(m, True) for m in machines])
186 yield self.save_state({})187 yield self.save_state({})
187 returnValue(deleted_machines)188 returnValue(deleted_machines)
@@ -200,8 +201,8 @@
200 """201 """
201 # XXX: need to actually handle errors as non-terminated machines202 # XXX: need to actually handle errors as non-terminated machines
202 # and not include them in the resulting list203 # and not include them in the resulting list
203 return gatherResults(204 return gather_results(
204 [self.shutdown_machine(m) for m in machines], consumeErrors=True)205 [self.shutdown_machine(m) for m in machines], consume_errors=True)
205206
206 def open_port(self, machine, machine_id, port, protocol="tcp"):207 def open_port(self, machine, machine_id, port, protocol="tcp"):
207 """Authorizes `port` using `protocol` on EC2 for `machine`."""208 """Authorizes `port` using `protocol` on EC2 for `machine`."""
208209
=== modified file 'juju/providers/openstack/tests/test_client.py'
--- juju/providers/openstack/tests/test_client.py 2012-08-28 12:25:41 +0000
+++ juju/providers/openstack/tests/test_client.py 2012-09-05 11:29:20 +0000
@@ -7,7 +7,6 @@
7from twisted.python.failure import Failure7from twisted.python.failure import Failure
8from twisted.web import (8from twisted.web import (
9 http_headers,9 http_headers,
10 client as txclient,
11 )10 )
1211
13from juju import errors12from juju import errors
@@ -142,7 +141,7 @@
142 "X-Server-Management-Url": ["http://testing.invalid/compute"],141 "X-Server-Management-Url": ["http://testing.invalid/compute"],
143 "X-Auth-Token": ["tok"],142 "X-Auth-Token": ["tok"],
144 }))143 }))
145 self.mocker.result(defer.fail(txclient.ResponseFailed([144 self.mocker.result(defer.fail(client.ResponseFailed([
146 Failure(self.SSLError()),145 Failure(self.SSLError()),
147 ])))146 ])))
148 self.mocker.replay()147 self.mocker.replay()

Subscribers

People subscribed via source and target branches

to status/vote changes: