Merge lp://qastaging/~johannes.erdfelt/nova/bug747394 into lp://qastaging/~hudson-openstack/nova/trunk

Would you mind renaming the "current" variable to something else? Or is there a reason it should be that way?

It's only named 'current' for consistency with other code (see add_to_xenstore and remove_from_xenstore).

I'm certainly not attached to that naming but I was hesitant to clean up the rest of the code at this point.

Would something like 'entries' or similar work?

> It's only named 'current' for consistency with other code (see add_to_xenstore
> and remove_from_xenstore).
>
> I'm certainly not attached to that naming but I was hesitant to clean up the
> rest of the code at this point.
>
> Would something like 'entries' or similar work?

Sure, that would work. I'm also not sure what official protocol on using the same variable name back to back like that is. Otherwise, this fix looks good

I've pushed up a change to rename 'current' to 'entries' so it's more descriptive

> 19 + for key in entries.keys():

Very minor, but it's more idiomatic to iterate directly over the `dict`:

    for key in entries:

Otherwise lgtm. Since the suggestion is so minor, marking as Approved anyway.

lgtm.

lgtm.

The attempt to merge lp:~johannes.erdfelt/nova/bug747394 into lp:nova failed. Below is the output from the failed tests.

AccountsTest
    test_account_create OK
    test_account_delete OK
    test_account_update OK
    test_get_account OK
AdminAPITest
    test_admin_disabled OK
    test_admin_enabled OK
APITest
    test_exceptions_are_converted_to_faults OK
Test
    test_authorize_token OK
    test_authorize_user OK
    test_bad_token OK
    test_bad_user_bad_key OK
    test_bad_user_good_key OK
    test_no_user OK
    test_token_expiry OK
TestFunctional
    test_token_doesnotexist OK
    test_token_expiry OK
TestLimiter
    test_authorize_token OK
LimiterTest
    test_limiter_custom_max_limit OK
    test_limiter_limit_and_offset OK
    test_limiter_limit_medium OK
    test_limiter_limit_over_max OK
    test_limiter_limit_zero OK
    test_limiter_negative_limit OK
    test_limiter_negative_offset OK
    test_limiter_nothing OK
    test_limiter_offset_bad OK
    test_limiter_offset_blank OK
    test_limiter_offset_medium OK
    test_limiter_offset_over_max OK
    test_limiter_offset_zero OK
ActionExtensionTest
    test_extended_action OK
    test_invalid_action OK
    test_invalid_action_body OK
ExtensionControllerTest
    test_get_by_alias OK
    test_index OK
ExtensionManagerTest
    test_get_resources OK
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr OK
    test_get_resources_with_stub_mgr OK
TestFaults
    test_400_fault_json OK
    test_400_fault_xml OK...

Hi Johannes!

Any update on this? Looks like this is the traceback causing all the errors in the test suite:

Traceback (most recent call last):
  File "/tmp/tmpv6MFQF/nova/tests/test_xenapi.py", line 503, in test_unrescue
    instance = self._create_instance()
  File "/tmp/tmpv6MFQF/nova/tests/test_xenapi.py", line 530, in _create_instance
    self.conn.spawn(instance)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi_conn.py", line 188, in spawn
    self._vmops.spawn(instance)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi/vmops.py", line 118, in spawn
    vm_ref = self._create_vm(instance, vdi_uuid, network_info)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi/vmops.py", line 178, in _create_vm
    self.inject_network_info(instance, network_info, vm_ref)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi/vmops.py", line 838, in inject_network_info
    self._inject_network_info(instance, network_info, vm_ref)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi/vmops.py", line 864, in _inject_network_info
    instance, location, vm_ref=vm_ref)
  File "/tmp/tmpv6MFQF/nova/virt/xenapi/vmops.py", line 972, in _make_plugin_call
    args = {'dom_id': vm_rec['domid'], 'path': path}
KeyError: 'domid'

Thoughts?
-jay

Please address the failing tests, then set back to "Needs Review."