OpenStack Compute (nova)

Merge lp:~jaypipes/nova/nova-virt-connection into lp:~hudson-openstack/nova/trunk

  1. nova-virt-connection
  2. Merge into trunk
Proposed by Jay Pipes
Status: Merged
Approved by: Eric Day
Approved revision: 219
Merged at revision: 261
Proposed branch: lp:~jaypipes/nova/nova-virt-connection
Merge into: lp:~hudson-openstack/nova/trunk
Prerequisite: lp:~jaypipes/nova/pylint
Diff against target: 230 lines (+63/-26)
2 files modified
nova/tests/api_unittest.py (+58/-25)
nova/virt/connection.py (+5/-1)
To merge this branch: bzr merge lp:~jaypipes/nova/nova-virt-connection
Reviewer Review Type Date Requested Status
Vish Ishaya (community) Approve
Review via email: mp+32208@code.launchpad.net

Description of the change

pylint fixes for /nova/virt/connection.py

To post a comment you must log in.
Revision history for this message
Vish Ishaya (vishvananda) wrote :
Download full text (9.6 KiB)

Not a huge deal, but pylint gives me a deprecation warning on disable-msg,
and says to use disable instead.

Vish

On Tue, Aug 10, 2010 at 7:26 AM, Jay Pipes <email address hidden> wrote:

> Jay Pipes has proposed merging lp:~jaypipes/nova/nova-virt-connection into
> lp:nova with lp:~jaypipes/nova/pylint as a prerequisite.
>
> Requested reviews:
> Nova Core (nova-core)
>
>
> pylint fixes for /nova/virt/connection.py
> --
> https://code.launchpad.net/~jaypipes/nova/nova-virt-connection/+merge/32208
> Your team Nova Core is requested to review the proposed merge of
> lp:~jaypipes/nova/nova-virt-connection into lp:nova.
>
> === modified file 'nova/tests/api_unittest.py'
> --- nova/tests/api_unittest.py 2010-07-27 04:07:28 +0000
> +++ nova/tests/api_unittest.py 2010-08-10 14:25:59 +0000
> @@ -16,6 +16,8 @@
> # License for the specific language governing permissions and
> limitations
> # under the License.
>
> +"""Unit tests for the API endpoint"""
> +
> import boto
> from boto.ec2 import regioninfo
> import httplib
> @@ -38,7 +40,15 @@
> # circuit boto calls and feed them into our tornado
> handlers,
> # it's pretty damn circuitous so apologies if you have to
> fix
> # a bug in it
> -def boto_to_tornado(method, path, headers, data, host, connection=None):
> +# NOTE(jaypipes) The pylint disables here are for R0913 (too many args)
> which
> +# isn't controllable since boto's HTTPRequest needs that
> many
> +# args, and for the version-differentiated import of
> tornado's
> +# httputil.
> +# NOTE(jaypipes): The disable-msg=E1101 and E1103 below is because pylint
> is
> +# unable to introspect the deferred's return value
> properly
> +
> +def boto_to_tornado(method, path, headers, data, # pylint:
> disable-msg=R0913
> + host, connection=None):
> """ translate boto requests into tornado requests
>
> connection should be a FakeTornadoHttpConnection instance
> @@ -46,7 +56,7 @@
> try:
> headers = httpserver.HTTPHeaders()
> except AttributeError:
> - from tornado import httputil
> + from tornado import httputil # pylint: disable-msg=E0611
> headers = httputil.HTTPHeaders()
> for k, v in headers.iteritems():
> headers[k] = v
> @@ -61,57 +71,64 @@
> return req
>
>
> -def raw_to_httpresponse(s):
> - """ translate a raw tornado http response into an httplib.HTTPResponse
> """
> - sock = FakeHttplibSocket(s)
> +def raw_to_httpresponse(response_string):
> + """translate a raw tornado http response into an
> httplib.HTTPResponse"""
> + sock = FakeHttplibSocket(response_string)
> resp = httplib.HTTPResponse(sock)
> resp.begin()
> return resp
>
>
> class FakeHttplibSocket(object):
> - """ a fake socket implementation for httplib.HTTPResponse, trivial """
> - def __init__(self, s):
> - self.fp = StringIO.StringIO(s)
> + """a fake socket implementation for httplib.HTTPResponse, trivial"""
> + def __init__(self, response_string):
> + self._buffer = StringIO.StringIO(response_string)
>
> - def makefile(self, mode, othe...

Read more...

Revision history for this message
Vish Ishaya (vishvananda) wrote :

lgtm, regardless

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

Must be a pylint version thing...I don't see the deprecation warning. Do you get it only on the block-level disable-msg at the top of the file? or all the disable-msg locations?

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Attempt to merge lp:~jaypipes/nova/nova-virt-connection into lp:nova failed due to merge conflicts:

text conflict in nova/virt/connection.py

Revision history for this message
Jay Pipes (jaypipes) wrote :

Conflicts resolved...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/tests/api_unittest.py'
2--- nova/tests/api_unittest.py 2010-07-27 04:07:28 +0000
3+++ nova/tests/api_unittest.py 2010-08-30 13:27:43 +0000
4@@ -16,6 +16,8 @@
5 # License for the specific language governing permissions and limitations
6 # under the License.
7
8+"""Unit tests for the API endpoint"""
9+
10 import boto
11 from boto.ec2 import regioninfo
12 import httplib
13@@ -38,7 +40,15 @@
14 # circuit boto calls and feed them into our tornado handlers,
15 # it's pretty damn circuitous so apologies if you have to fix
16 # a bug in it
17-def boto_to_tornado(method, path, headers, data, host, connection=None):
18+# NOTE(jaypipes) The pylint disables here are for R0913 (too many args) which
19+# isn't controllable since boto's HTTPRequest needs that many
20+# args, and for the version-differentiated import of tornado's
21+# httputil.
22+# NOTE(jaypipes): The disable-msg=E1101 and E1103 below is because pylint is
23+# unable to introspect the deferred's return value properly
24+
25+def boto_to_tornado(method, path, headers, data, # pylint: disable-msg=R0913
26+ host, connection=None):
27 """ translate boto requests into tornado requests
28
29 connection should be a FakeTornadoHttpConnection instance
30@@ -46,7 +56,7 @@
31 try:
32 headers = httpserver.HTTPHeaders()
33 except AttributeError:
34- from tornado import httputil
35+ from tornado import httputil # pylint: disable-msg=E0611
36 headers = httputil.HTTPHeaders()
37 for k, v in headers.iteritems():
38 headers[k] = v
39@@ -61,57 +71,64 @@
40 return req
41
42
43-def raw_to_httpresponse(s):
44- """ translate a raw tornado http response into an httplib.HTTPResponse """
45- sock = FakeHttplibSocket(s)
46+def raw_to_httpresponse(response_string):
47+ """translate a raw tornado http response into an httplib.HTTPResponse"""
48+ sock = FakeHttplibSocket(response_string)
49 resp = httplib.HTTPResponse(sock)
50 resp.begin()
51 return resp
52
53
54 class FakeHttplibSocket(object):
55- """ a fake socket implementation for httplib.HTTPResponse, trivial """
56- def __init__(self, s):
57- self.fp = StringIO.StringIO(s)
58+ """a fake socket implementation for httplib.HTTPResponse, trivial"""
59+ def __init__(self, response_string):
60+ self._buffer = StringIO.StringIO(response_string)
61
62- def makefile(self, mode, other):
63- return self.fp
64+ def makefile(self, _mode, _other):
65+ """Returns the socket's internal buffer"""
66+ return self._buffer
67
68
69 class FakeTornadoStream(object):
70- """ a fake stream to satisfy tornado's assumptions, trivial """
71- def set_close_callback(self, f):
72+ """a fake stream to satisfy tornado's assumptions, trivial"""
73+ def set_close_callback(self, _func):
74+ """Dummy callback for stream"""
75 pass
76
77
78 class FakeTornadoConnection(object):
79- """ a fake connection object for tornado to pass to its handlers
80+ """A fake connection object for tornado to pass to its handlers
81
82 web requests are expected to write to this as they get data and call
83 finish when they are done with the request, we buffer the writes and
84 kick off a callback when it is done so that we can feed the result back
85 into boto.
86 """
87- def __init__(self, d):
88- self.d = d
89+ def __init__(self, deferred):
90+ self._deferred = deferred
91 self._buffer = StringIO.StringIO()
92
93 def write(self, chunk):
94+ """Writes a chunk of data to the internal buffer"""
95 self._buffer.write(chunk)
96
97 def finish(self):
98- s = self._buffer.getvalue()
99- self.d.callback(s)
100+ """Finalizes the connection and returns the buffered data via the
101+ deferred callback.
102+ """
103+ data = self._buffer.getvalue()
104+ self._deferred.callback(data)
105
106 xheaders = None
107
108 @property
109- def stream(self):
110+ def stream(self): # pylint: disable-msg=R0201
111+ """Required property for interfacing with tornado"""
112 return FakeTornadoStream()
113
114
115 class FakeHttplibConnection(object):
116- """ a fake httplib.HTTPConnection for boto to use
117+ """A fake httplib.HTTPConnection for boto to use
118
119 requests made via this connection actually get translated and routed into
120 our tornado app, we then wait for the response and turn it back into
121@@ -123,7 +140,9 @@
122 self.deferred = defer.Deferred()
123
124 def request(self, method, path, data, headers):
125- req = boto_to_tornado
126+ """Creates a connection to a fake tornado and sets
127+ up a deferred request with the supplied data and
128+ headers"""
129 conn = FakeTornadoConnection(self.deferred)
130 request = boto_to_tornado(connection=conn,
131 method=method,
132@@ -131,12 +150,16 @@
133 headers=headers,
134 data=data,
135 host=self.host)
136- handler = self.app(request)
137+ self.app(request)
138 self.deferred.addCallback(raw_to_httpresponse)
139
140 def getresponse(self):
141+ """A bit of deferred magic for catching the response
142+ from the previously deferred request"""
143 @defer.inlineCallbacks
144 def _waiter():
145+ """Callback that simply yields the deferred's
146+ return value."""
147 result = yield self.deferred
148 defer.returnValue(result)
149 d = _waiter()
150@@ -144,14 +167,16 @@
151 # this deferred has already been called by the time
152 # we get here, we are going to cheat and return
153 # the result of the callback
154- return d.result
155+ return d.result # pylint: disable-msg=E1101
156
157 def close(self):
158+ """Required for compatibility with boto/tornado"""
159 pass
160
161
162 class ApiEc2TestCase(test.BaseTestCase):
163- def setUp(self):
164+ """Unit test for the cloud controller on an EC2 API"""
165+ def setUp(self): # pylint: disable-msg=C0103,C0111
166 super(ApiEc2TestCase, self).setUp()
167
168 self.manager = manager.AuthManager()
169@@ -171,12 +196,16 @@
170 self.mox.StubOutWithMock(self.ec2, 'new_http_connection')
171
172 def expect_http(self, host=None, is_secure=False):
173+ """Returns a new EC2 connection"""
174 http = FakeHttplibConnection(
175 self.app, '%s:%d' % (self.host, FLAGS.cc_port), False)
176+ # pylint: disable-msg=E1103
177 self.ec2.new_http_connection(host, is_secure).AndReturn(http)
178 return http
179
180 def test_describe_instances(self):
181+ """Test that, after creating a user and a project, the describe
182+ instances call to the API works properly"""
183 self.expect_http()
184 self.mox.ReplayAll()
185 user = self.manager.create_user('fake', 'fake', 'fake')
186@@ -187,14 +216,18 @@
187
188
189 def test_get_all_key_pairs(self):
190+ """Test that, after creating a user and project and generating
191+ a key pair, that the API call to list key pairs works properly"""
192 self.expect_http()
193 self.mox.ReplayAll()
194- keyname = "".join(random.choice("sdiuisudfsdcnpaqwertasd") for x in range(random.randint(4, 8)))
195+ keyname = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \
196+ for x in range(random.randint(4, 8)))
197 user = self.manager.create_user('fake', 'fake', 'fake')
198 project = self.manager.create_project('fake', 'fake', 'fake')
199 self.manager.generate_key_pair(user.id, keyname)
200
201 rv = self.ec2.get_all_key_pairs()
202- self.assertTrue(filter(lambda k: k.name == keyname, rv))
203+ results = [k for k in rv if k.name == keyname]
204+ self.assertEquals(len(results), 1)
205 self.manager.delete_project(project)
206 self.manager.delete_user(user)
207
208=== modified file 'nova/virt/connection.py'
209--- nova/virt/connection.py 2010-08-13 10:51:33 +0000
210+++ nova/virt/connection.py 2010-08-30 13:27:43 +0000
211@@ -17,6 +17,11 @@
212 # License for the specific language governing permissions and limitations
213 # under the License.
214
215+"""Abstraction of the underlying virtualization API"""
216+
217+import logging
218+import sys
219+
220 from nova import flags
221 from nova.virt import fake
222 from nova.virt import libvirt_conn
223@@ -35,7 +40,6 @@
224 Any object returned here must conform to the interface documented by
225 FakeConnection.
226 """
227-
228 # TODO(termie): maybe lazy load after initial check for permissions
229 # TODO(termie): check whether we can be disconnected
230 t = FLAGS.connection_type