From 1b89ce8b2981319b4849f6cef9a440e0f4625768 Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Tue, 3 Sep 2024 11:55:48 +0200 Subject: [PATCH 1/4] Run pep8 and functional test from tox Remove py35, py36 and add py38 to tox environments. Add enviroment for running pep8 and funcional tests. --- test-requirements.txt | 1 + tox.ini | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/test-requirements.txt b/test-requirements.txt index df4b868..8e8c95d 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -8,3 +8,4 @@ testrepository>=0.0.18 # Apache-2.0/BSD testtools>=1.4.0 # MIT os-testr>=0.8.0 # Apache-2.0 WebTest>=2.0 # MIT +stestr>=1.0.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 7b63590..58ce309 100644 --- a/tox.ini +++ b/tox.ini @@ -2,11 +2,14 @@ minversion = 2.0 # avoid sdist skipsdist = True -envlist = py35,py36,py37,pep8 +envlist = py37,py38,pep8 [testenv] usedevelop = True -install_command = {toxinidir}/tools/tox_install.sh {env:UPPER_CONSTRAINTS_FILE:https://raw.githubusercontent.com/sapcc/requirements/stable/rocky-m3/upper-constraints.txt} {opts} {packages} +deps = + -c{env:TOX_CONSTRAINTS_FILE:https://raw.githubusercontent.com/sapcc/requirements/stable/zed-m3/upper-constraints.txt} + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt setenv = VIRTUAL_ENV={envdir} BRANCH_NAME=master CLIENT_NAME=rate-limit @@ -15,11 +18,17 @@ setenv = VIRTUAL_ENV={envdir} OS_TEST_TIMEOUT=60 TESTS_DIR=./rate_limit/tests/ -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt +[testenv:functional] +setenv = + {[testenv]setenv} + OS_TEST_PATH=./rate_limit/tests commands = stestr run {posargs} [testenv:pep8] +allowlist_externals = flake8 +deps = + {[testenv]deps} + pylint==2.5.3 # GPLv2 commands = flake8 [testenv:venv] @@ -28,7 +37,7 @@ commands = {posargs} [flake8] ignore = D100,D101,D102,D103,D104,D203,D205,W605 show-source = True -exclude = .venv,.tox,dist,doc,*egg,build,test* +exclude = .git,__pycache__,docs/source/conf.py,old,build,dist,*.egg,.tox,venv,.venv max-line-length = 140 [testenv:docs] From 41690b1bbf8754cd6559faf6c80a25ebd131bd11 Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Tue, 3 Sep 2024 11:19:55 +0200 Subject: [PATCH 2/4] Fix test execution Two main issues caused the test execution to fail. Issue 1: The tests were failing due to a relative import not being found, resulting in the following error: ImportError: attempted relative import with no known parent package. Switching to absolute imports, as generally recommended, did not resolve the issue due to a circular reference. The problem arises because a Python file named rate_limit.py exists within the rate_limit module. When trying to import within the module using from rate_limit, it attempts to import itself, leading to the following error: ImportError: cannot import name 'backend' from partially initialized module 'rate_limit' (most likely due to a circular import). Issue 2: Middleware tests require a running Redis database to determine if requests should be rate limited via a Lua script. This commit mocks the result of the Lua script, eliminating the need for an active Redis instance during test execution. --- rate_limit/__init__.py | 2 +- rate_limit/{rate_limit.py => ratelimit.py} | 14 +-- rate_limit/tests/test_actiongroups.py | 2 +- .../tests/test_limesratelimitprovider.py | 2 +- rate_limit/tests/test_middleware.py | 108 ++++++++---------- 5 files changed, 60 insertions(+), 68 deletions(-) rename rate_limit/{rate_limit.py => ratelimit.py} (99%) diff --git a/rate_limit/__init__.py b/rate_limit/__init__.py index 8eed2dc..c0998a4 100644 --- a/rate_limit/__init__.py +++ b/rate_limit/__init__.py @@ -12,7 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from .rate_limit import OpenStackRateLimitMiddleware +from .ratelimit import OpenStackRateLimitMiddleware def main(global_config, **settings): diff --git a/rate_limit/rate_limit.py b/rate_limit/ratelimit.py similarity index 99% rename from rate_limit/rate_limit.py rename to rate_limit/ratelimit.py index 8d057a6..002c682 100644 --- a/rate_limit/rate_limit.py +++ b/rate_limit/ratelimit.py @@ -16,13 +16,13 @@ from datadog.dogstatsd import DogStatsd -from . import backend as rate_limit_backend -from . import common -from . import errors -from . import provider -from . import response -from . import units -from . import log +from rate_limit import backend as rate_limit_backend +from rate_limit import common +from rate_limit import errors +from rate_limit import provider +from rate_limit import response +from rate_limit import units +from rate_limit import log class OpenStackRateLimitMiddleware(object): diff --git a/rate_limit/tests/test_actiongroups.py b/rate_limit/tests/test_actiongroups.py index 0b1d788..e596205 100644 --- a/rate_limit/tests/test_actiongroups.py +++ b/rate_limit/tests/test_actiongroups.py @@ -15,7 +15,7 @@ import unittest import os -from rate_limit.rate_limit import OpenStackRateLimitMiddleware +from rate_limit.ratelimit import OpenStackRateLimitMiddleware from . import fake diff --git a/rate_limit/tests/test_limesratelimitprovider.py b/rate_limit/tests/test_limesratelimitprovider.py index 4d9abb1..52cfd1a 100644 --- a/rate_limit/tests/test_limesratelimitprovider.py +++ b/rate_limit/tests/test_limesratelimitprovider.py @@ -2,7 +2,7 @@ import os import json -from rate_limit.rate_limit import OpenStackRateLimitMiddleware, provider +from rate_limit import OpenStackRateLimitMiddleware, provider from . import fake WORKDIR = os.path.dirname(os.path.realpath(__file__)) diff --git a/rate_limit/tests/test_middleware.py b/rate_limit/tests/test_middleware.py index 09c33f6..4b29f91 100644 --- a/rate_limit/tests/test_middleware.py +++ b/rate_limit/tests/test_middleware.py @@ -16,7 +16,9 @@ import time import unittest -from rate_limit.rate_limit import OpenStackRateLimitMiddleware +from unittest.mock import patch, DEFAULT + +from rate_limit.ratelimit import OpenStackRateLimitMiddleware from rate_limit.response import BlacklistResponse from rate_limit.response import RateLimitExceededResponse from . import fake @@ -138,103 +140,93 @@ def test_get_rate_limit(self): "rate limit for '{0} {1}' should be '{2}' but got '{3}'".format(action, target_type_uri, expected_ratelimit, rate_limit) ) - def test_is_ratelimited_swift_local_container_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) + def test_is_ratelimited_swift_local_container_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 2r/m for target type URI account/container and action update. # Thus the first 2 requests should not be rate limited but the 3rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request should be a rate limit response - RateLimitExceededResponse( + expected = RateLimitExceededResponse( status='498 Rate Limited', body='Rate Limit Exceeded', - headerlist=[('X-Retry-After', 58), ('X-RateLimit-Retry-After', 58), - ('X-RateLimit-Limit', '2r/m'), ('X-RateLimit-Remaining', 0)] + headerlist=[('X-Retry-After', 58), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '2r/m'), + ('X-RateLimit-Remaining', remaining)] ) - ] - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) - def test_is_ratelimited_swift_local_wildcard_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) + def test_is_ratelimited_swift_local_wildcard_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container/foo_object' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 2r/m # for target type URI account/container and action update # which is targeted by wildcard pattern account/* # Thus the first 2 requests should not be rate limited but the 3rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request should be a rate limit response - RateLimitExceededResponse( + expected = RateLimitExceededResponse( status='498 Rate Limited', body='Rate Limit Exceeded', headerlist=[ - ('X-Retry-After', 58), - ('X-RateLimit-Retry-After', 58), + ('X-Retry-After', retry_after), + ('X-RateLimit-Retry-After', retry_after), ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', 0), + ('X-RateLimit-Remaining', remaining), ] ) - ] - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) - def test_is_ratelimited_swift_local_after_wildcard_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT,script_exists=DEFAULT) + def test_is_ratelimited_swift_local_after_wildcard_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container/foo_object/something/else' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 4r/m # for target type URI account/container/foo_object/something/else # and action update which is goes after wildcard pattern account/* # Thus the first 4 requests should not be rate limited but the 5rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request also not rate limited. - None, - # 4th request also not rate limited. - None, - # 5th request should be a rate limit response - RateLimitExceededResponse( + expected = RateLimitExceededResponse( status='498 Rate Limited', body='Rate Limit Exceeded', headerlist=[ ('X-Retry-After', 58), - ('X-RateLimit-Retry-After', 58), - ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', 0), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '4r/m'), + ('X-RateLimit-Remaining', remaining), ] - ) - ] - - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) - + ) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) def response_equal(expected, got): if isinstance(expected, (RateLimitExceededResponse, BlacklistResponse)) \ From f58ee86134ba5a57c60ded9b6c5d2120843b1bd9 Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Wed, 4 Sep 2024 16:07:52 +0200 Subject: [PATCH 3/4] Fix flake8 findings Fix flake8 findings and ignore D107 'Missing docstring in __init__'. --- rate_limit/common.py | 5 +-- rate_limit/log.py | 4 +- rate_limit/provider.py | 4 +- rate_limit/ratelimit.py | 4 +- rate_limit/tests/fake.py | 2 +- rate_limit/tests/test_actiongroups.py | 27 ++++++------ rate_limit/tests/test_middleware.py | 59 ++++++++++++++------------- rate_limit/tests/test_parse_config.py | 6 +-- rate_limit/tests/test_response.py | 22 ++++++---- rate_limit/tests/test_units.py | 2 +- rate_limit/utils.py | 2 +- tox.ini | 2 +- 12 files changed, 71 insertions(+), 68 deletions(-) diff --git a/rate_limit/common.py b/rate_limit/common.py index 2ba5eaf..35d863f 100644 --- a/rate_limit/common.py +++ b/rate_limit/common.py @@ -33,9 +33,8 @@ class Constants(object): - """ - Common constants used in various places. - """ + """Common constants used in various places.""" + ratelimit_response = 'ratelimit_response' blacklist_response = 'blacklist_response' max_sleep_time_seconds = 'max_sleep_time_seconds' diff --git a/rate_limit/log.py b/rate_limit/log.py index 7fa288b..c5ecb78 100644 --- a/rate_limit/log.py +++ b/rate_limit/log.py @@ -21,10 +21,8 @@ class Logger(object): - """ - Logger that attempts to log and ignores any error. + """Logger that attempts to log and ignores any error.""" - """ def __init__(self, name, product_name='rate_limit'): self.__logger = logging.getLogger(name) try: diff --git a/rate_limit/provider.py b/rate_limit/provider.py index 46e9d2c..e7f499b 100644 --- a/rate_limit/provider.py +++ b/rate_limit/provider.py @@ -120,7 +120,6 @@ def _get_wildcard_ratelimits(self, ratelimits, target_type_uri): :param target_type_uri: the target type URI of the request :return: target type uri ratelimits if exists """ - ttu_ratelimits = [] pattern_list = [ lr_key for lr_key in ratelimits @@ -134,14 +133,13 @@ def _get_wildcard_ratelimits(self, ratelimits, target_type_uri): def _match(self, uri, pattern_list): """ - Check if a URI matches to one of the patterns + Check if a URI matches to one of the patterns. :param uri: URI to check if it matches to one of the patterns :param pattern_list : patterns to match against the URI :return: True if path matches a pattern of the list and pattern as key for self.local_ratelimits. """ - for pattern in pattern_list: if uri.startswith(pattern[:-1]): return True, pattern diff --git a/rate_limit/ratelimit.py b/rate_limit/ratelimit.py index 002c682..1cbf7df 100644 --- a/rate_limit/ratelimit.py +++ b/rate_limit/ratelimit.py @@ -146,7 +146,7 @@ def __init__(self, app, **conf): self.logger.info("OpenStack Rate Limit Middleware ready for requests.") def _setup_response(self): - """Setup configurable RateLimitExceededResponse and BlacklistResponse.""" + """Set up configurable RateLimitExceededResponse and BlacklistResponse.""" # Default responses. ratelimit_response = response.RateLimitExceededResponse() blacklist_response = response.BlacklistResponse() @@ -185,7 +185,7 @@ def _setup_response(self): self.blacklist_response = blacklist_response def __setup_limes_ratelimit_provider(self): - """Setup Limes as provider for rate limits. If not successful fallback to configuration file.""" + """Set up Limes as provider for rate limits. If not successful fallback to configuration file.""" try: limes_ratelimit_provider = provider.LimesRateLimitProvider( service_type=self.service_type, diff --git a/rate_limit/tests/fake.py b/rate_limit/tests/fake.py index 4ec5370..ac92dc5 100644 --- a/rate_limit/tests/fake.py +++ b/rate_limit/tests/fake.py @@ -36,7 +36,7 @@ def incr(self, key, delta=1, time=0): def decr(self, key, delta=1, time=0): return self.incr(key, delta=-delta, time=time) - def delete(self,key): + def delete(self, key): try: del self.store[key] except KeyError: diff --git a/rate_limit/tests/test_actiongroups.py b/rate_limit/tests/test_actiongroups.py index e596205..a4fa90c 100644 --- a/rate_limit/tests/test_actiongroups.py +++ b/rate_limit/tests/test_actiongroups.py @@ -42,19 +42,18 @@ def test_groups(self): self.assertIsNotNone( rl_groups, "expected rate limit groups to be '{0}' but got '{1}'".format( -""" -groups: - write: - - update - - delete - - update/* - - delete/os-* - - read: - - read - - read/list - - read/*/list -""", + """ + groups: + write: + - update + - delete + - update/* + - delete/os-* + read: + - read + - read/list + - read/*/list + """, rl_groups ) ) @@ -95,7 +94,7 @@ def test_mapping(self): }, { 'action': 'read/rules/list', - 'expected':'read/rules/list', + 'expected': 'read/rules/list', }, ] diff --git a/rate_limit/tests/test_middleware.py b/rate_limit/tests/test_middleware.py index 4b29f91..af84dfa 100644 --- a/rate_limit/tests/test_middleware.py +++ b/rate_limit/tests/test_middleware.py @@ -13,7 +13,6 @@ # under the License. import os -import time import unittest from unittest.mock import patch, DEFAULT @@ -155,13 +154,13 @@ def test_is_ratelimited_swift_local_container_update(self, evalsha, script_exist # The current configuration as per /fixtures/swift.yaml allows 2r/m for target type URI account/container and action update. # Thus the first 2 requests should not be rate limited but the 3rd one. expected = RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[('X-Retry-After', 58), - ('X-RateLimit-Retry-After', retry_after), - ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', remaining)] - ) + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[('X-Retry-After', 58), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '2r/m'), + ('X-RateLimit-Remaining', remaining)] + ) result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) is_equal, msg = response_equal(expected, result) @@ -184,21 +183,21 @@ def test_is_ratelimited_swift_local_wildcard_update(self, evalsha, script_exists # which is targeted by wildcard pattern account/* # Thus the first 2 requests should not be rate limited but the 3rd one. expected = RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[ - ('X-Retry-After', retry_after), - ('X-RateLimit-Retry-After', retry_after), - ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', remaining), - ] - ) + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[ + ('X-Retry-After', retry_after), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '2r/m'), + ('X-RateLimit-Remaining', remaining), + ] + ) result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) is_equal, msg = response_equal(expected, result) self.assertTrue(is_equal, "test failed: {0}".format(msg)) - @patch.multiple('pyredis.Pool', evalsha=DEFAULT,script_exists=DEFAULT) + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) def test_is_ratelimited_swift_local_after_wildcard_update(self, evalsha, script_exists): scope = '123456' action = 'update' @@ -215,19 +214,21 @@ def test_is_ratelimited_swift_local_after_wildcard_update(self, evalsha, script_ # and action update which is goes after wildcard pattern account/* # Thus the first 4 requests should not be rate limited but the 5rd one. expected = RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[ - ('X-Retry-After', 58), - ('X-RateLimit-Retry-After', retry_after), - ('X-RateLimit-Limit', '4r/m'), - ('X-RateLimit-Remaining', remaining), - ] + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[ + ('X-Retry-After', 58), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '4r/m'), + ('X-RateLimit-Remaining', remaining), + ] ) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) is_equal, msg = response_equal(expected, result) self.assertTrue(is_equal, "test failed: {0}".format(msg)) + def response_equal(expected, got): if isinstance(expected, (RateLimitExceededResponse, BlacklistResponse)) \ and isinstance(got, (RateLimitExceededResponse, BlacklistResponse)): @@ -240,14 +241,14 @@ def response_equal(expected, got): return False, "expected status '{0}' but got '{1}'".format(expected.status, got.status) if expected.has_body and expected.body != got.body: - return False, "expected body '{0}' but got '{1}'".format(expected.body, got.body) + return False, "expected body '{0}' but got '{1}'".format(expected.body, got.body) if not expected.has_body and expected.json_body != got.json_body: - return False, "expected json body '{0}' but got '{1}'".format(expected.json_body, got.json_body) + return False, "expected json body '{0}' but got '{1}'".format(expected.json_body, got.json_body) return True, "items are equal" - if type(expected) != type(got): + if type(expected) is not type(got): return False, "expected type {0} but got type {1}".format(type(expected), type(got)) # Compare arguments if neither RateLimitResponse nor BlacklistResponse. diff --git a/rate_limit/tests/test_parse_config.py b/rate_limit/tests/test_parse_config.py index 0fa5a41..e7d9e1a 100644 --- a/rate_limit/tests/test_parse_config.py +++ b/rate_limit/tests/test_parse_config.py @@ -72,16 +72,16 @@ def test_load_swift_config(self): def test_parse_and_convert_to_per_seconds(self): stimuli = [ { - 'in': '5r/s', + 'in': '5r/s', 'expected': 5 }, { 'in': '1r/m', - 'expected': round(1/60.0, 4) + 'expected': round(1 / 60.0, 4) }, { 'in': '10r/d', - 'expected': round(1/8640.0,4) + 'expected': round(1 / 8640.0, 4) }, { 'in': '0.5r/s', diff --git a/rate_limit/tests/test_response.py b/rate_limit/tests/test_response.py index 86d4a4a..f31b2ba 100644 --- a/rate_limit/tests/test_response.py +++ b/rate_limit/tests/test_response.py @@ -1,6 +1,5 @@ import json import os -import six import unittest @@ -23,7 +22,8 @@ def test_default_ratelimitexceededresponse_json(self): self.assertEqual( ratelimit_response.content_type, common.Constants.content_type_json, - "expected response content type to be equal. want '{0}' but got '{1}'".format(common.Constants.content_type_json, ratelimit_response.content_type) + "expected response content type to be equal. want '{0}' but got '{1}'".format( + common.Constants.content_type_json, ratelimit_response.content_type) ) actual_body = sorted(json.loads(ratelimit_response.json_body)) @@ -39,7 +39,8 @@ def test_default_ratelimitexceededresponse_json(self): def test_custom_ratelimitexceededresponse_html(self): conf = common.load_config(SWIFTCONFIGPATH) - status, status_code, headers, body, json_body = response.response_parameters_from_config(conf.get(common.Constants.ratelimit_response)) + status, status_code, headers, body, json_body = response.response_parameters_from_config( + conf.get(common.Constants.ratelimit_response)) ratelimit_response = response.RateLimitExceededResponse( status=status, @@ -105,16 +106,19 @@ def test_default_blacklistresponse(self): .format(common.Constants.content_type_json, blacklist_response.content_type) ) - expected_json_body = json.dumps({"error": {"status": "497 Blacklisted", "message": "You have been blacklisted"}}, sort_keys=True) + expected_json_body = json.dumps( + {"error": {"status": "497 Blacklisted", "message": "You have been blacklisted"}}, sort_keys=True) self.assertEqual( blacklist_response.json_body, expected_json_body, - "expected blacklist response body to be '{0}' but got '{1}'".format(expected_json_body, blacklist_response.json_body) + "expected blacklist response body to be '{0}' but got '{1}'".format(expected_json_body, + blacklist_response.json_body) ) def test_custom_blacklistresponse_json(self): conf = common.load_config(SWIFTCONFIGPATH) - status, status_code, headers, body, json_body = response.response_parameters_from_config(conf.get(common.Constants.blacklist_response)) + status, status_code, headers, body, json_body = response.response_parameters_from_config( + conf.get(common.Constants.blacklist_response)) blacklist_response = response.BlacklistResponse( status=status, @@ -144,7 +148,11 @@ def test_custom_blacklistresponse_json(self): .format(common.Constants.content_type_json, blacklist_response.content_type) ) - expected_json_body = json.dumps({"error": {"status": "497 Blacklisted", "message": "You have been blacklisted. Please contact and administrator."}}, sort_keys=True) + expected_json_body = json.dumps( + {"error": {"status": "497 Blacklisted", + "message": "You have been blacklisted. Please contact and administrator."}}, + sort_keys=True) + self.assertEqual( blacklist_response.json_body, expected_json_body, diff --git a/rate_limit/tests/test_units.py b/rate_limit/tests/test_units.py index 06a9b88..7067cfe 100644 --- a/rate_limit/tests/test_units.py +++ b/rate_limit/tests/test_units.py @@ -35,7 +35,7 @@ def test_parse_sliding_window_rate_limit(self): }, { 'input': '100r/d', - 'expected': (100.0, 24*3600.0) + 'expected': (100.0, 24 * 3600.0) }, { 'input': '5r/2m', diff --git a/rate_limit/utils.py b/rate_limit/utils.py index c231af2..76f62db 100644 --- a/rate_limit/utils.py +++ b/rate_limit/utils.py @@ -11,7 +11,7 @@ def str_if_bytes(value): # NOTE: This function was copied from # https://github.com/andymccurdy/redis-py/blob/master/redis/client.py#L114 def parse_info(response): - "Parse the result of Redis's INFO command into a Python dict" + """Parse the result of Redis's INFO command into a Python dict.""" info = {} response = str_if_bytes(response) diff --git a/tox.ini b/tox.ini index 58ce309..403aa6a 100644 --- a/tox.ini +++ b/tox.ini @@ -35,7 +35,7 @@ commands = flake8 commands = {posargs} [flake8] -ignore = D100,D101,D102,D103,D104,D203,D205,W605 +ignore = D100,D101,D102,D103,D104,D107,D203,D205,W605 show-source = True exclude = .git,__pycache__,docs/source/conf.py,old,build,dist,*.egg,.tox,venv,.venv max-line-length = 140 From b6cae10ee7c6902dd733068b6d6141fd8793ed93 Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Tue, 3 Sep 2024 14:55:15 +0200 Subject: [PATCH 4/4] Add github action Github action runs tests and pep8 on each pull request to master. Updated flake8-docstring to latest version as this caused the action execution to fail. --- .github/workflows/tests.yaml | 38 ++++++++++++++++++++++++++++++++++++ test-requirements.txt | 2 +- tox.ini | 1 - 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/tests.yaml diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml new file mode 100644 index 0000000..880bfe1 --- /dev/null +++ b/.github/workflows/tests.yaml @@ -0,0 +1,38 @@ + # This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: Functional Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +env: + VIRTUALENV_PIP: "20.2.3" + +jobs: + build: + + #runs-on: ubuntu-latest + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + python: [3.8.14] + tox-env: [pep8, functional] + env: + TOXENV: ${{ matrix.tox-env }} + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python }} + - name: Install Tox and any other packages + run: | + pip install tox + - name: Running Tox + run: tox + diff --git a/test-requirements.txt b/test-requirements.txt index 8e8c95d..d608750 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,4 @@ -flake8-docstrings==0.2.1.post1 # MIT +flake8-docstrings==1.7.0 # MIT fixtures>=3.0.0 # Apache-2.0/BSD mock>=2.0.0 # BSD diff --git a/tox.ini b/tox.ini index 403aa6a..facac6f 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,6 @@ commands = stestr run {posargs} allowlist_externals = flake8 deps = {[testenv]deps} - pylint==2.5.3 # GPLv2 commands = flake8 [testenv:venv]