Correct thread handling in TranslationHook

TranslationHook was using a threading.local() to store an error
message however the threading.local() was being created in __init__
of the Hook. Hooks are not per request, so this wasn't really
working as planned.

Now, instead of using threading.local() at all, we just modify the
environ held by pecan (and later used by the
ParsableErrorMiddleware). environ is request-local.

This change feels a bit crufty because it is but because of the way
Exceptions are managed in the app there's not really any good way to do
it. We could consider changing the way Exceptions related to error
messages, but that would be a very large change.

Change-Id: I463059df28f291cea0644b1a9908a907f146ba1f
Closes-Bug: #1481244
This commit is contained in:
Chris Dent 2015-08-07 13:26:23 +00:00
parent a3d8780049
commit 9e07e7d971
2 changed files with 6 additions and 21 deletions

View File

@ -13,8 +13,6 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import threading
from oslo_policy import policy from oslo_policy import policy
from pecan import hooks from pecan import hooks
@ -45,17 +43,11 @@ class DBHook(hooks.PecanHook):
class TranslationHook(hooks.PecanHook): class TranslationHook(hooks.PecanHook):
def __init__(self):
# Use thread local storage to make this thread safe in situations
# where one pecan instance is being used to serve multiple request
# threads.
self.local_error = threading.local()
self.local_error.translatable_error = None
def before(self, state):
self.local_error.translatable_error = None
def after(self, state): def after(self, state):
# After a request has been done, we need to see if
# ClientSideError has added an error onto the response.
# If it has we need to get it info the thread-safe WSGI
# environ to be used by the ParsableErrorMiddleware.
if hasattr(state.response, 'translatable_error'): if hasattr(state.response, 'translatable_error'):
self.local_error.translatable_error = ( state.request.environ['translatable_error'] = (
state.response.translatable_error) state.response.translatable_error)

View File

@ -26,7 +26,6 @@ from oslo_log import log
import six import six
import webob import webob
from aodh.api import hooks
from aodh import i18n from aodh import i18n
from aodh.i18n import _ from aodh.i18n import _
@ -82,13 +81,7 @@ class ParsableErrorMiddleware(object):
app_iter = self.app(environ, replacement_start_response) app_iter = self.app(environ, replacement_start_response)
if (state['status_code'] // 100) not in (2, 3): if (state['status_code'] // 100) not in (2, 3):
req = webob.Request(environ) req = webob.Request(environ)
# Find the first TranslationHook in the array of hooks and use the error = environ.get('translatable_error')
# translatable_error object from it
error = None
for hook in self.app.hooks:
if isinstance(hook, hooks.TranslationHook):
error = hook.local_error.translatable_error
break
user_locale = self.best_match_language(req.accept_language) user_locale = self.best_match_language(req.accept_language)
if (req.accept.best_match(['application/json', 'application/xml']) if (req.accept.best_match(['application/json', 'application/xml'])
== 'application/xml'): == 'application/xml'):