From 00815820815ce70800b40340dc139fd0455e8e0b Mon Sep 17 00:00:00 2001 From: Robin Klingsberg Date: Sat, 16 Mar 2019 16:06:41 -0400 Subject: [PATCH 1/4] Move requirements files Add support for Tornado 6 --- .travis.yml | 2 +- HISTORY.rst | 6 ++++++ dev-requirements.txt | 5 ----- requirements.txt | 1 - requires/development.txt | 6 ++++++ requires/installation.txt | 1 + test-requirements.txt => requires/testing.txt | 0 setup.py | 4 ++-- sprockets/mixins/correlation/__init__.py | 2 +- tox.ini | 13 +++++++++---- 10 files changed, 26 insertions(+), 14 deletions(-) delete mode 100644 dev-requirements.txt delete mode 100644 requirements.txt create mode 100644 requires/development.txt create mode 100644 requires/installation.txt rename test-requirements.txt => requires/testing.txt (100%) diff --git a/.travis.yml b/.travis.yml index dd4eeed..449189d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ python: - 3.7 install: - pip install -e . - - pip install -r test-requirements.txt + - pip install -r requires/testing.txt - pip install tornado script: nosetests after_success: diff --git a/HISTORY.rst b/HISTORY.rst index f37329e..2b0655e 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,6 +1,11 @@ Version History --------------- +`2.0.1`_ (18-Mar-2019) +~~~~~~~~~~~~~~~~~~~~~~ +- Add support for Tornado 6 +- Move requirements files to requires/ + `2.0.0`_ (26-Nov-2018) ~~~~~~~~~~~~~~~~~~~~~~ - Drop support for Python 2.7, 3.3, 3.4 @@ -15,6 +20,7 @@ Version History ~~~~~~~~~~~~~~~~~~~~~~ - Adds ``sprockets.mixins.correlation.HandlerMixin`` +.. _`2.0.1`: https://github.com/sprockets/sprockets.mixins.correlation/compare/2.0.0...2.0.1 .. _`2.0.0`: https://github.com/sprockets/sprockets.mixins.correlation/compare/1.0.2...2.0.0 .. _`1.0.2`: https://github.com/sprockets/sprockets.mixins.correlation/compare/1.0.1...1.0.2 .. _`1.0.1`: https://github.com/sprockets/sprockets.mixins.correlation/compare/0.0.0...1.0.1 diff --git a/dev-requirements.txt b/dev-requirements.txt deleted file mode 100644 index 8870054..0000000 --- a/dev-requirements.txt +++ /dev/null @@ -1,5 +0,0 @@ --r requirements.txt --r test-requirements.txt -flake8 -sphinx>=1.2,<2 -sphinx-rtd-theme>=0.1,<1.0 diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index b33c704..0000000 --- a/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -tornado>=4.0,<5.2 diff --git a/requires/development.txt b/requires/development.txt new file mode 100644 index 0000000..314c854 --- /dev/null +++ b/requires/development.txt @@ -0,0 +1,6 @@ +-r requires/installation.txt +-r requires/testing.txt + +flake8 +sphinx>=1.2,<2 +sphinx-rtd-theme>=0.1,<1.0 diff --git a/requires/installation.txt b/requires/installation.txt new file mode 100644 index 0000000..ed4008b --- /dev/null +++ b/requires/installation.txt @@ -0,0 +1 @@ +tornado>=4.0,<6.1 diff --git a/test-requirements.txt b/requires/testing.txt similarity index 100% rename from test-requirements.txt rename to requires/testing.txt diff --git a/setup.py b/setup.py index c9c7f31..e596df6 100755 --- a/setup.py +++ b/setup.py @@ -51,8 +51,8 @@ setuptools.setup( ], packages=setuptools.find_packages(), namespace_packages=['sprockets'], - install_requires=read_requirements('requirements.txt'), - tests_require=read_requirements('test-requirements.txt'), + install_requires=read_requirements('installation.txt'), + tests_require=read_requirements('testing.txt'), test_suite='nose.collector', zip_safe=True, ) diff --git a/sprockets/mixins/correlation/__init__.py b/sprockets/mixins/correlation/__init__.py index 1611839..ff54cb0 100644 --- a/sprockets/mixins/correlation/__init__.py +++ b/sprockets/mixins/correlation/__init__.py @@ -7,5 +7,5 @@ except ImportError: raise ImportError -version_info = (2, 0, 0) +version_info = (2, 0, 1) __version__ = '.'.join(str(v) for v in version_info[:3]) diff --git a/tox.ini b/tox.ini index cba03ff..77ff7ee 100644 --- a/tox.ini +++ b/tox.ini @@ -1,20 +1,25 @@ [tox] -envlist = py35,py36,py37,tornado43,torando51 +envlist = py35,py36,py37,tornado43,torando51,tornado6 toxworkdir = {toxinidir}/build/tox skip_missing_intepreters = true [testenv] deps = - -rtest-requirements.txt + -r requires/testing.txt tornado commands = {envbindir}/nosetests [testenv:tornado43] deps = - -rtest-requirements.txt + -r requires/testing.txt tornado>=4.3,<4.4 [testenv:tornado51] deps = - -rtest-requirements.txt + -r requires/testing.txt tornado>=5.1,<5.2 + +[testenv:tornado6] +deps = + -r requires/testing.txt + tornado>=6,<6.1 From 4c26834f040e094d2b844fbf18da6758bf0bf270 Mon Sep 17 00:00:00 2001 From: Robin Klingsberg Date: Sat, 16 Mar 2019 16:07:27 -0400 Subject: [PATCH 2/4] Increase test coverage --- HISTORY.rst | 1 + sprockets/mixins/correlation/mixins.py | 2 +- tests.py | 90 +++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 2b0655e..3182a21 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -5,6 +5,7 @@ Version History ~~~~~~~~~~~~~~~~~~~~~~ - Add support for Tornado 6 - Move requirements files to requires/ +- Increase test coverage `2.0.0`_ (26-Nov-2018) ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/sprockets/mixins/correlation/mixins.py b/sprockets/mixins/correlation/mixins.py index 7703ec6..fba8d6c 100644 --- a/sprockets/mixins/correlation/mixins.py +++ b/sprockets/mixins/correlation/mixins.py @@ -46,7 +46,7 @@ class HandlerMixin(object): # one exists. We also want to set it in the outgoing response # which the property setter does for us. maybe_future = super(HandlerMixin, self).prepare() - if concurrent.is_future(maybe_future): + if concurrent.is_future(maybe_future): # pragma: nocover await maybe_future correlation_id = self.get_request_header(self.__header_name, None) diff --git a/tests.py b/tests.py index 72ac07f..a279fff 100644 --- a/tests.py +++ b/tests.py @@ -1,20 +1,28 @@ import uuid import unittest +import unittest.mock from tornado import testing, web from sprockets.mixins import correlation +from sprockets.mixins.correlation.mixins import correlation_id_logger -class CorrelatedRequestHandler(correlation.HandlerMixin, web.RequestHandler): +class RequestHandler(web.RequestHandler): def get(self, status_code): status_code = int(status_code) + self.set_status(status_code) + if status_code >= 300: raise web.HTTPError(status_code) self.write('status {0}'.format(status_code)) +class CorrelatedRequestHandler(correlation.HandlerMixin, RequestHandler): + pass + + class CorrelationMixinTests(testing.AsyncHTTPTestCase): def get_app(self): @@ -35,3 +43,83 @@ class CorrelationMixinTests(testing.AsyncHTTPTestCase): response = self.fetch('/status/500', headers={'Correlation-Id': correlation_id}) self.assertEqual(response.headers['correlation-id'], correlation_id) + + +class CorrelationIDLoggerTests(testing.AsyncHTTPTestCase): + + def get_app(self): + return web.Application([ + (r'/status/(?P\d+)', CorrelatedRequestHandler), + (r'/status/no-correlation/(?P\d+)', RequestHandler), + ], log_function=correlation_id_logger) + + def setUp(self): + self.patcher = unittest.mock.patch( + 'sprockets.mixins.correlation.mixins.log.access_log') + self.access_logger = self.patcher.start() + super(CorrelationIDLoggerTests, self).setUp() + + def tearDown(self): + self.patcher.stop() + super(CorrelationIDLoggerTests, self).tearDown() + + def test_lt_400_logs_info(self): + for status in (200, 202): + response = self.fetch('/status/{}'.format(status)) + + self.access_logger.info.assert_any_call( + "%d %s %.2fms {CID %s}", + status, + unittest.mock.ANY, + unittest.mock.ANY, + response.headers['correlation-id'] + ) + + def test_gte_400_lt_500_logs_warning(self): + for status in (400, 429): + response = self.fetch('/status/{}'.format(status)) + + self.access_logger.warning.assert_any_call( + "%d %s %.2fms {CID %s}", + status, + unittest.mock.ANY, + unittest.mock.ANY, + response.headers['correlation-id'] + ) + + def test_gte_500_logs_error(self): + for status in (500, 504): + response = self.fetch('/status/{}'.format(status)) + + self.access_logger.error.assert_any_call( + "%d %s %.2fms {CID %s}", + status, + unittest.mock.ANY, + unittest.mock.ANY, + response.headers['correlation-id'] + ) + + def test_uses_correlation_id_from_header_if_missing_from_handler(self): + correlation_id = uuid.uuid4().hex + + self.fetch('/status/no-correlation/200', + headers={'Correlation-Id': correlation_id}) + + self.access_logger.info.assert_any_call( + "%d %s %.2fms {CID %s}", + 200, + unittest.mock.ANY, + unittest.mock.ANY, + correlation_id + ) + + def test_correlation_id_is_none_if_missing_from_handler_and_header(self): + self.fetch('/status/no-correlation/200') + + self.access_logger.info.assert_any_call( + "%d %s %.2fms {CID %s}", + 200, + unittest.mock.ANY, + unittest.mock.ANY, + None + ) From 183eb3269ef35aa607d96e6722bb2f07afa0494e Mon Sep 17 00:00:00 2001 From: Robin Klingsberg Date: Tue, 19 Mar 2019 19:18:06 -0400 Subject: [PATCH 3/4] PR feedback --- requires/development.txt | 1 + requires/installation.txt | 2 +- sprockets/mixins/correlation/mixins.py | 2 +- tests.py | 4 ++-- tox.ini | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/requires/development.txt b/requires/development.txt index 314c854..6bd8b2d 100644 --- a/requires/development.txt +++ b/requires/development.txt @@ -1,5 +1,6 @@ -r requires/installation.txt -r requires/testing.txt +-e . flake8 sphinx>=1.2,<2 diff --git a/requires/installation.txt b/requires/installation.txt index ed4008b..44d8d5a 100644 --- a/requires/installation.txt +++ b/requires/installation.txt @@ -1 +1 @@ -tornado>=4.0,<6.1 +tornado>=4.3,<7 diff --git a/sprockets/mixins/correlation/mixins.py b/sprockets/mixins/correlation/mixins.py index fba8d6c..7703ec6 100644 --- a/sprockets/mixins/correlation/mixins.py +++ b/sprockets/mixins/correlation/mixins.py @@ -46,7 +46,7 @@ class HandlerMixin(object): # one exists. We also want to set it in the outgoing response # which the property setter does for us. maybe_future = super(HandlerMixin, self).prepare() - if concurrent.is_future(maybe_future): # pragma: nocover + if concurrent.is_future(maybe_future): await maybe_future correlation_id = self.get_request_header(self.__header_name, None) diff --git a/tests.py b/tests.py index a279fff..d5877b2 100644 --- a/tests.py +++ b/tests.py @@ -57,11 +57,11 @@ class CorrelationIDLoggerTests(testing.AsyncHTTPTestCase): self.patcher = unittest.mock.patch( 'sprockets.mixins.correlation.mixins.log.access_log') self.access_logger = self.patcher.start() - super(CorrelationIDLoggerTests, self).setUp() + super().setUp() def tearDown(self): self.patcher.stop() - super(CorrelationIDLoggerTests, self).tearDown() + super().tearDown() def test_lt_400_logs_info(self): for status in (200, 202): diff --git a/tox.ini b/tox.ini index 77ff7ee..713c902 100644 --- a/tox.ini +++ b/tox.ini @@ -22,4 +22,4 @@ deps = [testenv:tornado6] deps = -r requires/testing.txt - tornado>=6,<6.1 + tornado>=6,<7 From 076a91dfd63702071772e0b14ca392c50eb0ff22 Mon Sep 17 00:00:00 2001 From: Robin Klingsberg Date: Tue, 19 Mar 2019 19:18:17 -0400 Subject: [PATCH 4/4] Update copyright Fix env reference in tox.ini --- LICENSE | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/LICENSE b/LICENSE index 4fa05e6..7483ed3 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2015-2018 AWeber Communications +Copyright (c) 2015-2019 AWeber Communications All rights reserved. Redistribution and use in source and binary forms, with or without modification, diff --git a/tox.ini b/tox.ini index 713c902..6ce36fe 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py35,py36,py37,tornado43,torando51,tornado6 +envlist = py35,py36,py37,tornado43,tornado51,tornado6 toxworkdir = {toxinidir}/build/tox skip_missing_intepreters = true