Code review comment for lp:~gz/pyjuju/old_twisted_1029070

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 2012-09-01 14:46:03 +0000
+++ juju/providers/openstack/provider.py 2012-09-05 11:07:35 +0000
@@ -12,9 +12,10 @@

  import logging

-from twisted.internet.defer import inlineCallbacks, returnValue,
gatherResults
+from twisted.internet.defer import inlineCallbacks, returnValue

  from juju import errors
+from juju.lib.twistutils import gather_results
  from juju.lib.cache import CachedValue
  from juju.providers.common.base import MachineProviderBase

@@ -181,7 +182,7 @@
          :rtype: :class:`twisted.internet.defer.Deferred`
          """
          machines = yield self.get_machines()
- deleted_machines = yield gatherResults(
+ deleted_machines = yield gather_results(
              [self._delete_machine(m, True) for m in machines])
          yield self.save_state({})
          returnValue(deleted_machines)
@@ -200,8 +201,8 @@
          """
          # XXX: need to actually handle errors as non-terminated machines
          # and not include them in the resulting list
- return gatherResults(
- [self.shutdown_machine(m) for m in machines],
consumeErrors=True)
+ return gather_results(
+ [self.shutdown_machine(m) for m in machines],
consume_errors=True)

      def open_port(self, machine, machine_id, port, protocol="tcp"):
          """Authorizes `port` using `protocol` on EC2 for `machine`."""

Index: juju/providers/openstack/tests/test_client.py
=== 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:10:08 +0000
@@ -7,7 +7,6 @@
  from twisted.python.failure import Failure
  from twisted.web import (
      http_headers,
- client as txclient,
      )

  from juju import errors
@@ -142,7 +141,7 @@
              "X-Server-Management-Url": ["http://testing.invalid/compute"],
              "X-Auth-Token": ["tok"],
              }))
- self.mocker.result(defer.fail(txclient.ResponseFailed([
+ self.mocker.result(defer.fail(client.ResponseFailed([
              Failure(self.SSLError()),
              ])))
          self.mocker.replay()