Address all comments

logging/google/cloud/logging/handlers/_helpers.py

@@ -22,7 +22,7 @@
 except ImportError:
     flask = None
 
-from google.cloud.logging.handlers.middleware.request import _get_request
+from google.cloud.logging.handlers.middleware.request import _get_django_request
 
 _FLASK_TRACE_HEADER = 'X_CLOUD_TRACE_CONTEXT'
 _DJANGO_TRACE_HEADER = 'HTTP_X_CLOUD_TRACE_CONTEXT'
@@ -55,12 +55,12 @@ def get_trace_id_from_flask():
     :rtype: str
     :return: Trace_id in HTTP request headers.
     """
-    if not flask or not flask.request:
+    if flask is None or not flask.request:
         return None
 
     header = flask.request.headers.get(_FLASK_TRACE_HEADER)
 
-    if not header:
+    if header is None:
         return None
 
     trace_id = header.split('/')[0]
@@ -74,17 +74,17 @@ def get_trace_id_from_django():
     :rtype: str
     :return: Trace_id in HTTP request headers.
     """
-    request = _get_request()
+    request = _get_django_request()
 
-    if not request:
+    if request is None:
         return None
 
     try:
         header = request.META.get(_DJANGO_TRACE_HEADER)
     except KeyError:
         return None
 
-    if not header:
+    if header is None:
         return None
     trace_id = header.split('/')[0]
 
@@ -97,7 +97,7 @@ def get_trace_id():
     :rtype: str
     :returns: Trace_id in HTTP request headers.
     """
-    checkers = [get_trace_id_from_django, get_trace_id_from_flask]
+    checkers = (get_trace_id_from_django, get_trace_id_from_flask)
 
     for checker in checkers:
         trace_id = checker()

logging/google/cloud/logging/handlers/middleware/request.py

@@ -12,13 +12,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from threading import local
+import threading
 
 
-_thread_locals = local()
+_thread_locals = threading.local()
 
 
-def _get_request():
+def _get_django_request():
     """Get Django request from thread local.
 
     :rtype: str

logging/nox.py

@@ -31,7 +31,9 @@ def unit_tests(session, python_version):
     session.interpreter = 'python{}'.format(python_version)
 
     # Install all test dependencies, then install this package in-place.
-    session.install('mock', 'pytest', 'pytest-cov', 'flask', 'django', *LOCAL_DEPS)
+    session.install(
+        'mock', 'pytest', 'pytest-cov',
+        'flask', 'django', *LOCAL_DEPS)
     session.install('-e', '.')
 
     # Run py.test against the unit tests.

logging/tests/unit/handlers/middleware/test_request.py

@@ -25,24 +25,26 @@ def _get_target_class(self):
     def _make_one(self, *args, **kw):
         return self._get_target_class()(*args, **kw)
 
-    def setUp(self):
+    @classmethod
+    def setUpClass(cls):
         from django.conf import settings
         from django.test.utils import setup_test_environment
 
         if not settings.configured:
             settings.configure()
         setup_test_environment()
 
-    def tearDown(self):
+    @classmethod
+    def tearDownClass(cls):
         from django.test.utils import teardown_test_environment
 
         teardown_test_environment()
 
-    def test_get_request(self):
+    def test_get_django_request(self):
         from django.test import RequestFactory
-        from google.cloud.logging.handlers.middleware.request import _get_request
+        from google.cloud.logging.handlers.middleware.request import _get_django_request
 
-        self.middleware = self._make_one()
-        self.request = RequestFactory().get('/')
-        self.middleware.process_request(self.request)
-        self.assertEqual(_get_request(), self.request)
+        middleware = self._make_one()
+        request = RequestFactory().get('/')
+        middleware.process_request(request)
+        self.assertEqual(_get_django_request(), request)

logging/tests/unit/handlers/test__helpers.py

@@ -15,9 +15,10 @@
 import unittest
 
 
-class TestFlaskTrace(unittest.TestCase):
+class Test_get_trace_id_from_flask(unittest.TestCase):
 
-    def create_app(self):
+    @staticmethod
+    def create_app():
         import flask
 
         app = flask.Flask(__name__)
@@ -29,40 +30,41 @@ def index():
         return app
 
     def setUp(self):
-        self.app = self.create_app()
+        self.app = Test_get_trace_id_from_flask.create_app()
 
     def test_trace_id_no_context_header(self):
-        from google.cloud.logging.handlers._helpers import get_trace_id_from_flask
-        from google.cloud.logging.handlers._helpers import get_trace_id
+        from google.cloud.logging.handlers import _helpers
 
         with self.app.test_request_context(
                 path='/',
                 headers={}):
-            trace_id = get_trace_id_from_flask()
-            trace_id_returned = get_trace_id()
+            trace_id = _helpers.get_trace_id_from_flask()
+            trace_id_returned = _helpers.get_trace_id()
 
-        self.assertEqual(trace_id, None)
-        self.assertEqual(trace_id_returned, None)
+        self.assertIsNone(trace_id)
+        self.assertIsNone(trace_id_returned)
 
     def test_trace_id_valid_context_header(self):
         from google.cloud.logging.handlers._helpers import get_trace_id_from_flask
         from google.cloud.logging.handlers._helpers import get_trace_id
 
         flask_trace_header = 'X_CLOUD_TRACE_CONTEXT'
-        flask_trace_id = 'testtraceidflask/testspanid'
+        expected_trace_id = 'testtraceidflask'
+        flask_trace_id = expected_trace_id + '/testspanid'
 
-        with self.app.test_request_context(
-                path='/',
-                headers={flask_trace_header:flask_trace_id}):
+        context = self.app.test_request_context(
+            path='/',
+            headers={flask_trace_header: flask_trace_id})
+
+        with context:
             trace_id = get_trace_id_from_flask()
             trace_id_returned = get_trace_id()
 
-        expected_trace_id = 'testtraceidflask'
         self.assertEqual(trace_id, expected_trace_id)
         self.assertEqual(trace_id, trace_id_returned)
 
 
-class TestDjangoTrace(unittest.TestCase):
+class Test_get_trace_id_from_django(unittest.TestCase):
 
     def setUp(self):
         from django.conf import settings
@@ -79,22 +81,20 @@ def tearDown(self):
 
     def test_trace_id_no_context_header(self):
         from django.test import RequestFactory
-        from google.cloud.logging.handlers._helpers import get_trace_id_from_django
-        from google.cloud.logging.handlers._helpers import get_trace_id
-        from google.cloud.logging.handlers.middleware.request import RequestMiddleware
-        from google.cloud.logging.handlers.middleware.request import _thread_locals
+        from google.cloud.logging.handlers import _helpers
+        from google.cloud.logging.handlers.middleware import request
 
-        request = RequestFactory().get('/')
+        django_request = RequestFactory().get('/')
 
-        middleware = RequestMiddleware()
-        middleware.process_request(request)
-        trace_id = get_trace_id_from_django()
-        trace_id_returned = get_trace_id()
+        middleware = request.RequestMiddleware()
+        middleware.process_request(django_request)
+        trace_id = _helpers.get_trace_id_from_django()
+        trace_id_returned = _helpers.get_trace_id()
 
-        self.assertEqual(trace_id, None)
-        self.assertEqual(trace_id_returned, None)
+        self.assertIsNone(trace_id)
+        self.assertIsNone(trace_id_returned)
 
-        _thread_locals.__dict__.clear()
+        request._thread_locals.__dict__.clear()
 
     def test_trace_id_valid_context_header(self):
         from django.test import RequestFactory
@@ -108,7 +108,7 @@ def test_trace_id_valid_context_header(self):
 
         request = RequestFactory().get(
             '/',
-            **{django_trace_header:django_trace_id})
+            **{django_trace_header: django_trace_id})
 
         middleware = RequestMiddleware()
         middleware.process_request(request)

logging/tests/unit/handlers/test_handlers.py

@@ -45,7 +45,9 @@ def test_emit(self):
                                    None, None)
         handler.emit(record)
 
-        self.assertEqual(handler.transport.send_called_with, (record, message, _GLOBAL_RESOURCE, None))
+        self.assertEqual(
+            handler.transport.send_called_with,
+            (record, message, _GLOBAL_RESOURCE, None))
 
 
 class TestSetupLogging(unittest.TestCase):

logging/tests/unit/handlers/transports/test_background_thread.py

@@ -63,7 +63,8 @@ def test_send(self):
 
         transport.send(record, message, _GLOBAL_RESOURCE, None)
 
-        transport.worker.enqueue.assert_called_once_with(record, message, _GLOBAL_RESOURCE, None)
+        transport.worker.enqueue.assert_called_once_with(
+            record, message, _GLOBAL_RESOURCE, None)
 
     def test_flush(self):
         client = _Client(self.PROJECT)

logging/tests/unit/handlers/transports/test_sync.py

@@ -63,7 +63,8 @@ class _Logger(object):
     def __init__(self, name):
         self.name = name
 
-    def log_struct(self, message, severity=None, resource=_GLOBAL_RESOURCE, labels=None):
+    def log_struct(self, message, severity=None,
+                   resource=_GLOBAL_RESOURCE, labels=None):
         self.log_struct_called_with = (message, severity, resource, labels)