diff --git a/HACKING.rst b/HACKING.rst index c1e0588f..2440afa4 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -20,6 +20,8 @@ Manila Specific Commandments - [M330] LOG.warning messages require translations _LW()! - [M331] Log messages require translations! - [M333] 'oslo_' should be used instead of 'oslo.' +- [M336] Must use a dict comprehension instead of a dict constructor + with a sequence of key-value pairs. LOG Translations diff --git a/manila/api/v1/limits.py b/manila/api/v1/limits.py index eaaee175..3d484119 100644 --- a/manila/api/v1/limits.py +++ b/manila/api/v1/limits.py @@ -80,7 +80,7 @@ class Limit(object): 60 * 60 * 24: "DAY", } - UNIT_MAP = dict([(v, k) for k, v in UNITS.items()]) + UNIT_MAP = {v: k for k, v in UNITS.items()} def __init__(self, verb, uri, regex, value, unit): """Initialize a new `Limit`. diff --git a/manila/api/v1/share_snapshots.py b/manila/api/v1/share_snapshots.py index c7c4dbf7..a071282c 100644 --- a/manila/api/v1/share_snapshots.py +++ b/manila/api/v1/share_snapshots.py @@ -154,9 +154,9 @@ class ShareSnapshotsController(wsgi.Controller, wsgi.AdminActionsMixin): 'display_description', ) - update_dict = dict([(key, snapshot_data[key]) - for key in valid_update_keys - if key in snapshot_data]) + update_dict = {key: snapshot_data[key] + for key in valid_update_keys + if key in snapshot_data} try: snapshot = self.share_api.get_snapshot(context, id) diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index b424ea84..d9b3d56f 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -197,9 +197,9 @@ class ShareMixin(object): 'is_public', ) - update_dict = dict([(key, share_data[key]) - for key in valid_update_keys - if key in share_data]) + update_dict = {key: share_data[key] + for key in valid_update_keys + if key in share_data} try: share = self.share_api.get(context, id) diff --git a/manila/api/v2/quota_sets.py b/manila/api/v2/quota_sets.py index bd9e38a1..cbe089d5 100644 --- a/manila/api/v2/quota_sets.py +++ b/manila/api/v2/quota_sets.py @@ -63,7 +63,7 @@ class QuotaSetsMixin(object): if usages: return values - return dict((k, v['limit']) for k, v in values.items()) + return {k: v['limit'] for k, v in values.items()} @wsgi.Controller.authorize("show") def _show(self, req, id): diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index 09582142..c9548548 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -50,7 +50,7 @@ class ViewBuilder(common.ViewBuilder): context = request.environ['manila.context'] metadata = share.get('share_metadata') if metadata: - metadata = dict((item['key'], item['value']) for item in metadata) + metadata = {item['key']: item['value'] for item in metadata} else: metadata = {} diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 29843072..abfcd8a7 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -770,7 +770,7 @@ def _get_user_quota_usages(context, session, project_id, user_id): models.QuotaUsage.user_id is None)).\ with_lockmode('update').\ all() - return dict((row.resource, row) for row in rows) + return {row.resource: row for row in rows} def _get_project_quota_usages(context, session, project_id): @@ -980,8 +980,8 @@ def quota_reserve(context, resources, project_quotas, user_quotas, deltas, usages = project_usages else: usages = user_usages - usages = dict((k, dict(in_use=v['in_use'], reserved=v['reserved'])) - for k, v in usages.items()) + usages = {k: dict(in_use=v['in_use'], reserved=v['reserved']) + for k, v in usages.items()} raise exception.OverQuota(overs=sorted(overs), quotas=user_quotas, usages=usages) @@ -2465,7 +2465,7 @@ def driver_private_data_get(context, host, entity_id, key=None, query = _driver_private_data_query(session, context, host, entity_id, key) if key is None or isinstance(key, list): - return dict([(item.key, item.value) for item in query.all()]) + return {item.key: item.value for item in query.all()} else: result = query.first() return result["value"] if result is not None else default @@ -2606,8 +2606,8 @@ to a single dict: 'extra_specs' : {'k1': 'v1'} """ inst_type_dict = dict(inst_type_query) - extra_specs = dict([(x['key'], x['value']) - for x in inst_type_query['extra_specs']]) + extra_specs = {x['key']: x['value'] + for x in inst_type_query['extra_specs']} inst_type_dict['extra_specs'] = extra_specs return inst_type_dict diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index b949c7c3..4bf98e43 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -698,8 +698,8 @@ class ShareServer(BASE, ManilaBase): @property def backend_details(self): - return dict((model['key'], model['value']) - for model in self._backend_details) + return {model['key']: model['value'] + for model in self._backend_details} _extra_keys = ['backend_details'] diff --git a/manila/hacking/checks.py b/manila/hacking/checks.py index 10a46925..79026154 100644 --- a/manila/hacking/checks.py +++ b/manila/hacking/checks.py @@ -53,6 +53,7 @@ underscore_import_check = re.compile(r"(.)*import _(.)*") # We need this for cases where they have created their own _ function. custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)") +dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") class BaseASTChecker(ast.NodeVisitor): @@ -232,6 +233,14 @@ def check_oslo_namespace_imports(logical_line, physical_line, filename): yield(0, msg) +def dict_constructor_with_list_copy(logical_line): + msg = ("M336: Must use a dict comprehension instead of a dict constructor" + " with a sequence of key-value pairs." + ) + if dict_constructor_with_list_copy_re.match(logical_line): + yield (0, msg) + + def factory(register): register(validate_log_translations) register(check_explicit_underscore_import) @@ -239,3 +248,4 @@ def factory(register): register(CheckForStrExc) register(CheckForTransAdd) register(check_oslo_namespace_imports) + register(dict_constructor_with_list_copy) diff --git a/manila/network/neutron/api.py b/manila/network/neutron/api.py index 514df5db..332d4e86 100644 --- a/manila/network/neutron/api.py +++ b/manila/network/neutron/api.py @@ -218,7 +218,7 @@ class API(object): def list_extensions(self): extensions_list = self.client.list_extensions().get('extensions') - return dict((ext['name'], ext) for ext in extensions_list) + return {ext['name']: ext for ext in extensions_list} def _has_port_binding_extension(self): if not self.extensions: diff --git a/manila/quota.py b/manila/quota.py index 159e3a3c..b508b2ff 100644 --- a/manila/quota.py +++ b/manila/quota.py @@ -310,8 +310,8 @@ class DbQuotaDriver(object): else: sync_filt = lambda x: not hasattr(x, 'sync') desired = set(keys) - sub_resources = dict((k, v) for k, v in resources.items() - if k in desired and sync_filt(v)) + sub_resources = {k: v for k, v in resources.items() + if k in desired and sync_filt(v)} # Make sure we accounted for all of them... if len(keys) != len(sub_resources): @@ -330,7 +330,7 @@ class DbQuotaDriver(object): context.quota_class, usages=False) - return dict((k, v['limit']) for k, v in quotas.items()) + return {k: v['limit'] for k, v in quotas.items()} def limit_check(self, context, resources, values, project_id=None, user_id=None): diff --git a/manila/share/manager.py b/manila/share/manager.py index 0e1a6968..e6583f75 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -1173,8 +1173,8 @@ class ShareManager(manager.SchedulerDependentManager): """Get info about relationships between pools and share_servers.""" share_servers = self.db.share_server_get_all_by_host(context, self.host) - return dict((server['id'], self.driver.get_share_server_pools(server)) - for server in share_servers) + return {server['id']: self.driver.get_share_server_pools(server) + for server in share_servers} @add_hooks @utils.require_driver_initialized diff --git a/manila/test.py b/manila/test.py index d5bab7ce..216dba1b 100644 --- a/manila/test.py +++ b/manila/test.py @@ -344,8 +344,8 @@ class TestCase(base_test.BaseTestCase): def _dict_from_object(self, obj, ignored_keys): if ignored_keys is None: ignored_keys = [] - return dict([(k, v) for k, v in obj.iteritems() - if k not in ignored_keys]) + return {k: v for k, v in obj.iteritems() + if k not in ignored_keys} def _assertEqualListsOfObjects(self, objs1, objs2, ignored_keys=None): obj_to_dict = lambda o: self._dict_from_object(o, ignored_keys) diff --git a/manila/tests/share/test_drivers_private_data.py b/manila/tests/share/test_drivers_private_data.py index 333ce672..864c88c5 100644 --- a/manila/tests/share/test_drivers_private_data.py +++ b/manila/tests/share/test_drivers_private_data.py @@ -135,7 +135,7 @@ def create_arg_list(key_names): def create_arg_dict(key_names): - return dict((key, fake_storage_data[key]) for key in key_names) + return {key: fake_storage_data[key] for key in key_names} @ddt.ddt diff --git a/manila/tests/test_hacking.py b/manila/tests/test_hacking.py index a3c22179..a63b7298 100644 --- a/manila/tests/test_hacking.py +++ b/manila/tests/test_hacking.py @@ -200,3 +200,31 @@ class HackingTestCase(test.TestCase): """ errors = [] self._assert_has_errors(code, checker, expected_errors=errors) + + def test_dict_constructor_with_list_copy(self): + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict([(i, connect_info[i])")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " attrs = dict([(k, _from_json(v))")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " type_names = dict((value, key) for key, value in")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict((value, key) for key, value in")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + "foo(param=dict((k, v) for k, v in bar.items()))")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dict([[i,i] for i in range(3)])")))) + + self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy( + " dd = dict([i,i] for i in range(3))")))) + + self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( + " create_kwargs = dict(snapshot=snapshot,")))) + + self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( + " self._render_dict(xml, data_el, data.__dict__)")))) diff --git a/manila/tests/test_quota.py b/manila/tests/test_quota.py index 5a9b24d9..694a28d4 100644 --- a/manila/tests/test_quota.py +++ b/manila/tests/test_quota.py @@ -952,8 +952,8 @@ class DbQuotaDriverTestCase(test.TestCase): quota_class=None, defaults=True, usages=True): self.calls.append('get_project_quotas') - return dict((k, dict(limit=v.default)) - for k, v in resources.items()) + return {k: dict(limit=v.default) + for k, v in resources.items()} self.mock_object(self.driver, 'get_project_quotas', fake_get_project_quotas)