Merge lp:~jk0/nova/lp744833 into lp:~hudson-openstack/nova/trunk

Proposed by Josh Kearney
Status: Merged
Approved by: Josh Kearney
Approved revision: 924
Merged at revision: 924
Proposed branch: lp:~jk0/nova/lp744833
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 53 lines (+13/-5)
2 files modified
nova/virt/vmwareapi/vim.py (+11/-4)
nova/virt/vmwareapi_conn.py (+2/-1)
To merge this branch: bzr merge lp:~jk0/nova/lp744833
Reviewer Review Type Date Requested Status
termie (community) Approve
Rick Clark (community) Approve
Review via email: mp+55604@code.launchpad.net

Description of the change

Avoid any hard dependencies in nova.virt.vmwareapi.vim.

To post a comment you must log in.
Revision history for this message
Rick Clark (dendrobates) wrote :

lgtm

review: Approve
Revision history for this message
Brian Lamar (blamar) wrote :

I feel like this should be headed off at the vim.py level where suds is actually used. Maybe it's just me, but I'd rather:

try:
    import suds
except ImportError:
    suds = None

and then check in the Vim class that suds is not None?

Revision history for this message
Josh Kearney (jk0) wrote :

> I feel like this should be headed off at the vim.py level where suds is
> actually used. Maybe it's just me, but I'd rather:
>
> try:
> import suds
> except ImportError:
> suds = None
>
> and then check in the Vim class that suds is not None?

I actually started off doing it that way, but the problem is that there are a half-dozen or so other imports from suds. All of the import try-excepts felt very dirty.

This way avoids almost all of the refactoring that would otherwise have to be done (which I am a fan of).

Revision history for this message
termie (termie) wrote :

I agree with brian on this one, I think this is the wrong place to address the issue

review: Needs Fixing
lp:~jk0/nova/lp744833 updated
923. By Josh Kearney

Handle in vim.py

924. By Josh Kearney

Whoops

Revision history for this message
termie (termie) wrote :

while technically this should be importing modules not classes, i'll let that slide so you don't have to go change their entire file.

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/virt/vmwareapi/vim.py'
--- nova/virt/vmwareapi/vim.py 2011-03-24 16:38:31 +0000
+++ nova/virt/vmwareapi/vim.py 2011-03-30 18:58:35 +0000
@@ -21,10 +21,14 @@
2121
22import httplib22import httplib
2323
24from suds import WebFault24try:
25from suds.client import Client25 suds = True
26from suds.plugin import MessagePlugin26 from suds import WebFault
27from suds.sudsobject import Property27 from suds.client import Client
28 from suds.plugin import MessagePlugin
29 from suds.sudsobject import Property
30except ImportError:
31 suds = False
2832
29from nova import flags33from nova import flags
30from nova.virt.vmwareapi import error_util34from nova.virt.vmwareapi import error_util
@@ -75,6 +79,9 @@
75 protocol: http or https79 protocol: http or https
76 host : ESX IPAddress[:port] or ESX Hostname[:port]80 host : ESX IPAddress[:port] or ESX Hostname[:port]
77 """81 """
82 if not suds:
83 raise Exception(_("Unable to import suds."))
84
78 self._protocol = protocol85 self._protocol = protocol
79 self._host_name = host86 self._host_name = host
80 wsdl_url = FLAGS.vmwareapi_wsdl_loc87 wsdl_url = FLAGS.vmwareapi_wsdl_loc
8188
=== modified file 'nova/virt/vmwareapi_conn.py'
--- nova/virt/vmwareapi_conn.py 2011-03-24 16:38:31 +0000
+++ nova/virt/vmwareapi_conn.py 2011-03-30 18:58:35 +0000
@@ -47,6 +47,7 @@
47from nova.virt.vmwareapi import vim_util47from nova.virt.vmwareapi import vim_util
48from nova.virt.vmwareapi.vmops import VMWareVMOps48from nova.virt.vmwareapi.vmops import VMWareVMOps
4949
50
50LOG = logging.getLogger("nova.virt.vmwareapi_conn")51LOG = logging.getLogger("nova.virt.vmwareapi_conn")
5152
52FLAGS = flags.FLAGS53FLAGS = flags.FLAGS
@@ -109,7 +110,7 @@
109 def __init__(self, host_ip, host_username, host_password,110 def __init__(self, host_ip, host_username, host_password,
110 api_retry_count, scheme="https"):111 api_retry_count, scheme="https"):
111 session = VMWareAPISession(host_ip, host_username, host_password,112 session = VMWareAPISession(host_ip, host_username, host_password,
112 api_retry_count, scheme=scheme)113 api_retry_count, scheme=scheme)
113 self._vmops = VMWareVMOps(session)114 self._vmops = VMWareVMOps(session)
114115
115 def init_host(self, host):116 def init_host(self, host):