diff --git a/HACKING.rst b/HACKING.rst index c14cc901d0..f450b181da 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -9,3 +9,4 @@ Magnum Specific Commandments --------------------------- - [M301] policy.enforce_wsgi decorator must be the first decorator on a method. +- [M322] Method's default argument shouldn't be mutable. diff --git a/magnum/api/middleware/auth_token.py b/magnum/api/middleware/auth_token.py index 5302a679eb..3fb5900de2 100644 --- a/magnum/api/middleware/auth_token.py +++ b/magnum/api/middleware/auth_token.py @@ -29,7 +29,9 @@ class AuthTokenMiddleware(auth_token.AuthProtocol): for public routes in the API. """ - def __init__(self, app, conf, public_api_routes=[]): + def __init__(self, app, conf, public_api_routes=None): + if public_api_routes is None: + public_api_routes = [] route_pattern_tpl = '%s(\.json|\.xml)?$' try: diff --git a/magnum/common/service.py b/magnum/common/service.py index 22ff1e6cc2..79c7bcd051 100644 --- a/magnum/common/service.py +++ b/magnum/common/service.py @@ -16,7 +16,9 @@ from oslo_config import cfg from oslo_log import log as logging -def prepare_service(argv=[]): +def prepare_service(argv=None): + if argv is None: + argv = [] logging.register_options(cfg.CONF) cfg.CONF(argv[1:], project='magnum') logging.setup(cfg.CONF, 'magnum') diff --git a/magnum/db/sqlalchemy/api.py b/magnum/db/sqlalchemy/api.py index 62ee30236c..d0dca7a6e3 100644 --- a/magnum/db/sqlalchemy/api.py +++ b/magnum/db/sqlalchemy/api.py @@ -106,7 +106,9 @@ class Connection(api.Connection): def __init__(self): pass - def _add_tenant_filters(self, context, query, opts={}): + def _add_tenant_filters(self, context, query, opts=None): + if opts is None: + opts = {} all_tenants = opts.get('get_all_tenants', False) @@ -146,7 +148,9 @@ class Connection(api.Connection): return query def get_bay_list(self, context, filters=None, limit=None, marker=None, - sort_key=None, sort_dir=None, opts={}): + sort_key=None, sort_dir=None, opts=None): + if opts is None: + opts = {} query = model_query(models.Bay) query = self._add_tenant_filters(context, query, opts=opts) query = self._add_bays_filters(query, filters) diff --git a/magnum/hacking/checks.py b/magnum/hacking/checks.py index 3de3b25bf0..8b12d2b1ab 100644 --- a/magnum/hacking/checks.py +++ b/magnum/hacking/checks.py @@ -32,6 +32,7 @@ Guidelines for writing new hacking checks enforce_re = re.compile(r"@policy.enforce_wsgi*") decorator_re = re.compile(r"@.*") +mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") def check_policy_enforce_decorator(logical_line, @@ -44,5 +45,12 @@ def check_policy_enforce_decorator(logical_line, yield(0, msg) +def no_mutable_default_args(logical_line): + msg = "M322: Method's default argument shouldn't be mutable!" + if mutable_default_args.match(logical_line): + yield (0, msg) + + def factory(register): register(check_policy_enforce_decorator) + register(no_mutable_default_args) diff --git a/magnum/tests/unit/api/base.py b/magnum/tests/unit/api/base.py index ff1be21cba..dcc97de932 100644 --- a/magnum/tests/unit/api/base.py +++ b/magnum/tests/unit/api/base.py @@ -189,7 +189,8 @@ class FunctionalTest(base.DbTestCase): return response def get_json(self, path, expect_errors=False, headers=None, - extra_environ=None, q=[], path_prefix=PATH_PREFIX, **params): + extra_environ=None, q=None, path_prefix=PATH_PREFIX, + **params): """Sends simulated HTTP GET request to Pecan test app. :param path: url path of target service @@ -203,6 +204,8 @@ class FunctionalTest(base.DbTestCase): :param path_prefix: prefix of the url path :param params: content for wsgi.input of request """ + if q is None: + q = [] full_path = path_prefix + path query_params = {'q.field': [], 'q.value': [], diff --git a/magnum/tests/unit/test_hacking.py b/magnum/tests/unit/test_hacking.py index b0b3d7d593..d927137332 100644 --- a/magnum/tests/unit/test_hacking.py +++ b/magnum/tests/unit/test_hacking.py @@ -83,3 +83,13 @@ class HackingTestCase(base.TestCase): """ self._assert_has_errors(code, checks.check_policy_enforce_decorator, expected_errors=[(2, 0, "M301")]) + + def test_no_mutable_default_args(self): + self.assertEqual(1, len(list(checks.no_mutable_default_args( + "def get_info_from_bdm(virt_type, bdm, mapping=[])")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined = []")))) + + self.assertEqual(0, len(list(checks.no_mutable_default_args( + "defined, undefined = [], {}"))))