Merge lp:~tpatil/nova/bug699654 into lp:~hudson-openstack/nova/trunk

[Tushar Patil]

Fix for LP Bug #699654

Fix for LP Bug #699654

[Devin Carlen (devcamcar)]

20 - LOG.audit(_("Authentication Failure: %s"), str(ex))
21 + LOG.error(_("Authentication Failure: %s"), ex.args[0])

Why was this changed from audit to error?

review: Needs Information

[Tushar Patil (tpatil)]

My mistake, I will make revert it back to audit.

[Tushar Patil (tpatil)]

Please review and let me know your comments.

[Devin Carlen (devcamcar)]

approve

review: Approve

[Vish Ishaya (vishvananda)]

lgtm.

review: Approve

[Tushar Patil (tpatil)]

Request to merge this branch into lp:nova if there are no more review comments.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~tpatil/nova/bug699654 into lp:nova failed. Below is the output from the failed tests.

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 ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    test_ge...

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~tpatil/nova/bug699654 into lp:nova failed. Below is the output from the failed tests.

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 ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    test_ge...

[Vish Ishaya (vishvananda)]

still has pep8 issues.

Vish

On Jan 25, 2011, at 3:24 PM, OpenStack Hudson wrote:

> The proposal to merge lp:~tpatil/nova/bug699654 into lp:nova has been updated.
>
> Status: Approved => Needs review
>
> For more details, see:
> https://code.launchpad.net/~tpatil/nova/bug699654/+merge/47008
> --
> https://code.launchpad.net/~tpatil/nova/bug699654/+merge/47008
> You are reviewing the proposed merge of lp:~tpatil/nova/bug699654 into lp:nova.

[Tushar Patil (tpatil)]

I already check for pep8 errors and it returned me following

root@ubuntu-network-api-server:/home/tpatil/bugs/bug699654# pep8 .
./doc/ext/nova_autodoc.py:8:1: E302 expected 2 blank lines, found 1
./doc/ext/nova_todo.py:2:80: E501 line too long (88 characters)
./doc/ext/nova_todo.py:25:76: W291 trailing whitespace
./doc/ext/nova_todo.py:27:5: E303 too many blank lines (2)
./doc/ext/nova_todo.py:29:63: E702 multiple statements on one line (semicolon)
./doc/ext/nova_todo.py:29:57: E231 missing whitespace after ','
./doc/ext/nova_todo.py:62:1: W293 blank line contains whitespace
./doc/ext/nova_todo.py:74:34: E701 multiple statements on one line (colon)
./doc/ext/nova_todo.py:82:27: E225 missing whitespace around operator
./doc/ext/nova_todo.py:101:1: W391 blank line at end of file
./doc/source/conf.py:14:11: E401 multiple imports on one line
./doc/source/conf.py:42:3: E111 indentation is not a multiple of four
./smoketests/user_smoketests.py:244:21: W601 .has_key() is deprecated, use 'in'
./tools/ajaxterm/ajaxterm.py:23:5: E301 expected 1 blank line, found 0
./tools/ajaxterm/ajaxterm.py:64:65: E203 whitespace before ':'
./tools/ajaxterm/qweb.py:718:22: E201 whitespace after '('

I didn't see any issues reported in my code, so I was relaxed. But it seems like I need to first run merge, check pep8 for errors and then push the branch again.

[Vish Ishaya (vishvananda)]

the command is pep8 --repeat nova bin/* (or just ./run_tests.sh) the bin files don't get picked up automatically because they don't end in .py

You also may have an old version of pep8

Vish

On Jan 25, 2011, at 3:38 PM, Tushar Patil wrote:

> I already check for pep8 errors and it returned me following
>
> root@ubuntu-network-api-server:/home/tpatil/bugs/bug699654# pep8 .
> ./doc/ext/nova_autodoc.py:8:1: E302 expected 2 blank lines, found 1
> ./doc/ext/nova_todo.py:2:80: E501 line too long (88 characters)
> ./doc/ext/nova_todo.py:25:76: W291 trailing whitespace
> ./doc/ext/nova_todo.py:27:5: E303 too many blank lines (2)
> ./doc/ext/nova_todo.py:29:63: E702 multiple statements on one line (semicolon)
> ./doc/ext/nova_todo.py:29:57: E231 missing whitespace after ','
> ./doc/ext/nova_todo.py:62:1: W293 blank line contains whitespace
> ./doc/ext/nova_todo.py:74:34: E701 multiple statements on one line (colon)
> ./doc/ext/nova_todo.py:82:27: E225 missing whitespace around operator
> ./doc/ext/nova_todo.py:101:1: W391 blank line at end of file
> ./doc/source/conf.py:14:11: E401 multiple imports on one line
> ./doc/source/conf.py:42:3: E111 indentation is not a multiple of four
> ./smoketests/user_smoketests.py:244:21: W601 .has_key() is deprecated, use 'in'
> ./tools/ajaxterm/ajaxterm.py:23:5: E301 expected 1 blank line, found 0
> ./tools/ajaxterm/ajaxterm.py:64:65: E203 whitespace before ':'
> ./tools/ajaxterm/qweb.py:718:22: E201 whitespace after '('
>
> I didn't see any issues reported in my code, so I was relaxed. But it seems like I need to first run merge, check pep8 for errors and then push the branch again.
> --
> https://code.launchpad.net/~tpatil/nova/bug699654/+merge/47008
> You are reviewing the proposed merge of lp:~tpatil/nova/bug699654 into lp:nova.

[Vish Ishaya (vishvananda)]

vishvananda@i-uswrlzxk:~/friendly-db$ pep8 --version
0.6.1

[Tushar Patil (tpatil)]

I have correct pep8 version.
Now I can see the above error when I run the command pep8 -repeat nova bin/*
I will fix it soon. Sorry to bother you again and again.

I am learning lots of new things in this project. Will ensure this problem doesn't happen again.

[Vish Ishaya (vishvananda)]

Don't worry about it. We're all learning.

Vish

On Jan 25, 2011, at 3:49 PM, Tushar Patil wrote:

> I have correct pep8 version.
> Now I can see the above error when I run the command pep8 -repeat nova bin/*
> I will fix it soon. Sorry to bother you again and again.
>
> I am learning lots of new things in this project. Will ensure this problem doesn't happen again.
>
>
> --
> https://code.launchpad.net/~tpatil/nova/bug699654/+merge/47008
> You are reviewing the proposed merge of lp:~tpatil/nova/bug699654 into lp:nova.

[Vish Ishaya (vishvananda)]

As a side note, you might want to configure your editor to automatically strip white-space from the end of the lines on save.

[lp:~tpatil/nova/bug699654]

602. By Tushar Patil on 2011-01-25

Fixed pep8 errors

=== modified file 'Authors'
=== modified file 'nova/api/ec2/__init__.py'
--- nova/api/ec2/__init__.py	2011-01-19 02:00:28 +0000
+++ nova/api/ec2/__init__.py	2011-01-25 23:57:16 +0000
@@ -170,7 +170,7 @@
                     req.path)
         # Be explicit for what exceptions are 403, the rest bubble as 500
         except (exception.NotFound, exception.NotAuthorized) as ex:
-            LOG.audit(_("Authentication Failure: %s"), str(ex))
+            LOG.audit(_("Authentication Failure: %s"), ex.args[0])
             raise webob.exc.HTTPForbidden()
 
         # Authenticated!
@@ -314,17 +314,18 @@
         try:
             result = api_request.invoke(context)
         except exception.NotFound as ex:
-            LOG.info(_('NotFound raised: %s'), str(ex),  context=context)
-            return self._error(req, context, type(ex).__name__, str(ex))
+            LOG.info(_('NotFound raised: %s'), ex.args[0],  context=context)
+            return self._error(req, context, type(ex).__name__, ex.args[0])
         except exception.ApiError as ex:
-            LOG.exception(_('ApiError raised: %s'), str(ex), context=context)
+            LOG.exception(_('ApiError raised: %s'), ex.args[0],
+                          context=context)
             if ex.code:
-                return self._error(req, context, ex.code, str(ex))
+                return self._error(req, context, ex.code, ex.args[0])
             else:
-                return self._error(req, context, type(ex).__name__, str(ex))
+                return self._error(req, context, type(ex).__name__, ex.args[0])
         except Exception as ex:
             extra = {'environment': req.environ}
-            LOG.exception(_('Unexpected error raised: %s'), str(ex),
+            LOG.exception(_('Unexpected error raised: %s'), ex.args[0],
                           extra=extra, context=context)
             return self._error(req,
                                context,
@@ -347,7 +348,8 @@
                          '<Response><Errors><Error><Code>%s</Code>'
                          '<Message>%s</Message></Error></Errors>'
                          '<RequestID>%s</RequestID></Response>' %
-                         (code, message, context.request_id))
+                         (utils.utf8(code), utils.utf8(message),
+                         utils.utf8(context.request_id)))
         return resp