Merge lp:~soren/nova/include-test-extensions into lp:~hudson-openstack/nova/trunk

[Soren Hansen]

Toss an __init__ in the test extensions dir. This gets it included in the tarball.

Make extensions code ignore modules whose name starts with '_'.

Warn if an extension doesn't define a class or factory by the right name.

Resubmitting, since it's suddenly more of an actual patch.

[Sandy Walsh (sandy-walsh)]

sure

review: Approve

[Jay Pipes (jaypipes)]

lgtm.

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/include-test-extensions 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 ERROR
    test_invalid_action ERROR
    test_invalid_action_body ERROR
ExtensionControllerTest
    test_get_by_alias ERROR
    test_index ERROR
ExtensionManagerTest
    test_get_resources ERROR
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr ERROR
    test_get_resources_with_stub_mgr OK
TestFaults
    test_fault_parts OK
    test_raise ...

[Soren Hansen (soren)]

:(

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/include-test-extensions 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 ERROR
    test_invalid_action ERROR
    test_invalid_action_body ERROR
ExtensionControllerTest
    test_get_by_alias ERROR
    test_index ERROR
ExtensionManagerTest
    test_get_resources ERROR
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr ERROR
    test_get_resources_with_stub_mgr OK
TestFaults
    test_fault_parts OK
    test_raise ...

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/include-test-extensions 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 ERROR
    test_invalid_action ERROR
    test_invalid_action_body ERROR
ExtensionControllerTest
    test_get_by_alias ERROR
    test_index ERROR
ExtensionManagerTest
    test_get_resources ERROR
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr ERROR
    test_get_resources_with_stub_mgr OK
TestFaults
    test_fault_parts OK
    test_raise ...

[Soren Hansen (soren)]

This makes *no* sense to me.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/include-test-extensions 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 ERROR
    test_invalid_action ERROR
    test_invalid_action_body ERROR
ExtensionControllerTest
    test_get_by_alias ERROR
    test_index ERROR
ExtensionManagerTest
    test_get_resources ERROR
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr ERROR
    test_get_resources_with_stub_mgr OK
TestFaults
    test_fault_parts OK
    test_raise ...

[Soren Hansen (soren)]

Stab in the dark: Added contents (license and copyright) to the file. Trying again.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/include-test-extensions 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 ERROR
    test_invalid_action ERROR
    test_invalid_action_body ERROR
ExtensionControllerTest
    test_get_by_alias ERROR
    test_index ERROR
ExtensionManagerTest
    test_get_resources ERROR
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr ERROR
    test_get_resources_with_stub_mgr OK
TestFaults
    test_fault_parts OK
    test_raise ...

[termie (termie)]

neat bug

[Soren Hansen (soren)]

Ok, I *swear* I ran the test suite before uploading this. I'm *almost*
sure I even ran it in the right directory.

[Jay Pipes (jaypipes)]

funny little bug. good patience tracking this down.

review: Approve

[Sandy Walsh (sandy-walsh)]

Is the try-except block still required? If so, which operation throws the AttributeError?

LOG.warn(_('Did not find expected name "%(ext_name)" in %(ext_path)s') % locals())

If load_source fails the entire service fails, correct? (not really related to this patch, but still a concern)

Personally I think I'd rather see an except Exception, e: to log the failure and press on. But others may disagree?

review: Needs Fixing

[Soren Hansen (soren)]

> Personally I think I'd rather see an except Exception, e: to log the failure
> and press on. But others may disagree?

Personally, to be frank, I don't give a crap. I'm just trying to get usable tarballs again. Just tell me what to do, I'll do it.

[Vish Ishaya (vishvananda)]

I vote for getting this in as is. Soren has been fighting with this one for far too long

review: Approve

[Soren Hansen (soren)]

> I vote for getting this in as is. Soren has been fighting with this one for
> far too long

Admittedly, most of the wait has been my fault, but thanks for the approve nonetheless.

[Dan Prince (dan-prince)]

Hey Soren,

Sorry that I missed this in my initial extensions work BTW. Didn't realize we needed the extra __init__.py files for tarball happiness, etc.

Anyway. Just tried your branch.

Got a pep8 error:

[dan.prince@dovetail nova-include-test-extensions]$ pep8 nova/api/openstack/extensions.py
nova/api/openstack/extensions.py:327:31: E201 whitespace after '{'

[Dan Prince (dan-prince)]

Otherwise this looks good to me. Anxious to have PPA working again too!

review: Approve

[Sandy Walsh (sandy-walsh)]

/me caves to the pressure :)

review: Approve

[Soren Hansen (soren)]

/me applies JIT pep8

[OpenStack Infra (hudson-openstack)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'nova/api/openstack/extensions.py'
--- nova/api/openstack/extensions.py	2011-03-21 18:00:39 +0000
+++ nova/api/openstack/extensions.py	2011-03-28 20:23:27 +0000
@@ -317,16 +317,19 @@
             LOG.audit(_('Loading extension file: %s'), f)
             mod_name, file_ext = os.path.splitext(os.path.split(f)[-1])
             ext_path = os.path.join(self.path, f)
-            if file_ext.lower() == '.py':
+            if file_ext.lower() == '.py' and not mod_name.startswith('_'):
                 mod = imp.load_source(mod_name, ext_path)
                 ext_name = mod_name[0].upper() + mod_name[1:]
-                try:
-                    new_ext = getattr(mod, ext_name)()
-                    self._check_extension(new_ext)
-                    self.extensions[new_ext.get_alias()] = new_ext
-                except AttributeError as ex:
-                    LOG.exception(_("Exception loading extension: %s"),
-                                   unicode(ex))
+                new_ext_class = getattr(mod, ext_name, None)
+                if not new_ext_class:
+                    LOG.warn(_('Did not find expected name '
+                               '"%(ext_name)" in %(file)s'),
+                             {'ext_name': ext_name,
+                             'file': ext_path})
+                    continue
+                new_ext = new_ext_class()
+                self._check_extension(new_ext)
+                self.extensions[new_ext.get_alias()] = new_ext
 
 
 class ResponseExtension(object):

=== added file 'nova/tests/api/openstack/extensions/__init__.py'
--- nova/tests/api/openstack/extensions/__init__.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/openstack/extensions/__init__.py	2011-03-28 20:23:27 +0000
@@ -0,0 +1,15 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.