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
1=== modified file 'juju/providers/openstack/client.py'
2--- juju/providers/openstack/client.py 2012-09-03 11:08:13 +0000
3+++ juju/providers/openstack/client.py 2012-09-05 11:29:20 +0000
4@@ -30,10 +30,13 @@
5 from twisted.internet.interfaces import IProducer
6 from twisted.internet import reactor
7
8-from twisted.web import (
9- client,
10- http_headers,
11- )
12+from twisted.web.client import Agent
13+# Older twisted versions don't expose _newclient exceptions via client module
14+try:
15+ from twisted.web.client import ResponseFailed
16+except ImportError:
17+ from twisted.web._newclient import ResponseFailed
18+from twisted.web.http_headers import Headers
19 from zope.interface import implements
20
21 from juju import errors
22@@ -100,7 +103,7 @@
23 def _translate_response_failed(failure):
24 """Turn internal twisted client failures into juju exceptions"""
25 txerr = failure.value
26- if isinstance(txerr, client.ResponseFailed):
27+ if isinstance(txerr, ResponseFailed):
28 for reason in txerr.reasons:
29 err = reason.value
30 if isinstance(err, SSLError):
31@@ -110,7 +113,7 @@
32
33 @inlineCallbacks
34 def request(method, url, extra_headers=(), body=None, check_certs=False):
35- headers = http_headers.Headers({
36+ headers = Headers({
37 # GZ 2012-07-03: Previously passed Accept: application/json header
38 # here, but not always the right thing. Bad for swift?
39 "User-Agent": [_USER_AGENT],
40@@ -128,7 +131,7 @@
41 kwargs = {}
42 if check_certs:
43 kwargs['contextFactory'] = WebVerifyingContextFactory()
44- agent = client.Agent(reactor, **kwargs)
45+ agent = Agent(reactor, **kwargs)
46 response = yield agent.request(method, url, headers, body).addErrback(
47 _translate_response_failed)
48 if response.length == 0:
49
50=== modified file 'juju/providers/openstack/provider.py'
51--- juju/providers/openstack/provider.py 2012-09-01 14:46:03 +0000
52+++ juju/providers/openstack/provider.py 2012-09-05 11:29:20 +0000
53@@ -12,9 +12,10 @@
54
55 import logging
56
57-from twisted.internet.defer import inlineCallbacks, returnValue, gatherResults
58+from twisted.internet.defer import inlineCallbacks, returnValue
59
60 from juju import errors
61+from juju.lib.twistutils import gather_results
62 from juju.lib.cache import CachedValue
63 from juju.providers.common.base import MachineProviderBase
64
65@@ -181,7 +182,7 @@
66 :rtype: :class:`twisted.internet.defer.Deferred`
67 """
68 machines = yield self.get_machines()
69- deleted_machines = yield gatherResults(
70+ deleted_machines = yield gather_results(
71 [self._delete_machine(m, True) for m in machines])
72 yield self.save_state({})
73 returnValue(deleted_machines)
74@@ -200,8 +201,8 @@
75 """
76 # XXX: need to actually handle errors as non-terminated machines
77 # and not include them in the resulting list
78- return gatherResults(
79- [self.shutdown_machine(m) for m in machines], consumeErrors=True)
80+ return gather_results(
81+ [self.shutdown_machine(m) for m in machines], consume_errors=True)
82
83 def open_port(self, machine, machine_id, port, protocol="tcp"):
84 """Authorizes `port` using `protocol` on EC2 for `machine`."""
85
86=== modified file 'juju/providers/openstack/tests/test_client.py'
87--- juju/providers/openstack/tests/test_client.py 2012-08-28 12:25:41 +0000
88+++ juju/providers/openstack/tests/test_client.py 2012-09-05 11:29:20 +0000
89@@ -7,7 +7,6 @@
90 from twisted.python.failure import Failure
91 from twisted.web import (
92 http_headers,
93- client as txclient,
94 )
95
96 from juju import errors
97@@ -142,7 +141,7 @@
98 "X-Server-Management-Url": ["http://testing.invalid/compute"],
99 "X-Auth-Token": ["tok"],
100 }))
101- self.mocker.result(defer.fail(txclient.ResponseFailed([
102+ self.mocker.result(defer.fail(client.ResponseFailed([
103 Failure(self.SSLError()),
104 ])))
105 self.mocker.replay()

Subscribers

People subscribed via source and target branches

to status/vote changes: