diff --git a/HACKING.rst b/HACKING.rst index 769d76efa3..76a5f69002 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -22,4 +22,5 @@ glance Specific Commandments - [G324] Validate that LOG.error messages use _LE. - [G325] Validate that LOG.critical messages use _LC. - [G326] Validate that LOG.warning messages use _LW. -- [G327] Prevent use of deprecated contextlib.nested \ No newline at end of file +- [G327] Prevent use of deprecated contextlib.nested +- [G328] Must use a dict comprehension instead of a dict constructor with a sequence of key-value pairs diff --git a/glance/common/client.py b/glance/common/client.py index cc72c4574f..87d97eab72 100644 --- a/glance/common/client.py +++ b/glance/common/client.py @@ -413,8 +413,7 @@ class BaseClient(object): names and values """ to_str = encodeutils.safe_encode - return dict([(to_str(h), to_str(v)) for h, v in - six.iteritems(headers)]) + return {to_str(h): to_str(v) for h, v in six.iteritems(headers)} @handle_redirects def _do_request(self, method, url, body, headers): diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index cf20ca01d5..59773cca32 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -454,7 +454,7 @@ def _make_conditions_from_filters(filters, is_public=None): tag_filters.extend([models.ImageTag.value == tag]) tag_conditions.append(tag_filters) - filters = dict([(k, v) for k, v in filters.items() if v is not None]) + filters = {k: v for k, v in filters.items() if v is not None} for (k, v) in filters.items(): key = k diff --git a/glance/hacking/checks.py b/glance/hacking/checks.py index bb41e7c251..66a88666ba 100644 --- a/glance/hacking/checks.py +++ b/glance/hacking/checks.py @@ -55,6 +55,7 @@ log_translation_critical = re.compile( r"(.)*LOG\.(critical)\(\s*(_\(|'|\")") log_translation_warning = re.compile( r"(.)*LOG\.(warning)\(\s*(_\(|'|\")") +dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") def assert_true_instance(logical_line): @@ -148,6 +149,13 @@ def check_no_contextlib_nested(logical_line): yield(0, msg) +def dict_constructor_with_list_copy(logical_line): + msg = ("G328: 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(assert_true_instance) register(assert_equal_type) @@ -156,3 +164,4 @@ def factory(register): register(no_direct_use_of_unicode_function) register(validate_log_translations) register(check_no_contextlib_nested) + register(dict_constructor_with_list_copy) diff --git a/glance/image_cache/drivers/sqlite.py b/glance/image_cache/drivers/sqlite.py index 74c31736f4..ab77abab76 100644 --- a/glance/image_cache/drivers/sqlite.py +++ b/glance/image_cache/drivers/sqlite.py @@ -83,8 +83,7 @@ class SqliteConnection(sqlite3.Connection): def dict_factory(cur, row): - return dict( - ((col[0], row[idx]) for idx, col in enumerate(cur.description))) + return {col[0]: row[idx] for idx, col in enumerate(cur.description)} class Driver(base.Driver): diff --git a/glance/registry/api/v1/images.py b/glance/registry/api/v1/images.py index ff7b535437..6329b81592 100644 --- a/glance/registry/api/v1/images.py +++ b/glance/registry/api/v1/images.py @@ -466,8 +466,8 @@ class Controller(object): try: LOG.debug("Updating image %(id)s with metadata: %(image_data)r", {'id': id, - 'image_data': dict((k, v) for k, v in image_data.items() - if k != 'locations')}) + 'image_data': {k: v for k, v in image_data.items() + if k != 'locations'}}) image_data = _normalize_image_location_for_db(image_data) if purge_props == "true": purge_props = True @@ -532,14 +532,13 @@ def make_image_dict(image): """ def _fetch_attrs(d, attrs): - return dict([(a, d[a]) for a in attrs - if a in d.keys()]) + return {a: d[a] for a in attrs if a in d.keys()} # TODO(sirp): should this be a dict, or a list of dicts? # A plain dict is more convenient, but list of dicts would provide # access to created_at, etc - properties = dict((p['name'], p['value']) - for p in image['properties'] if not p['deleted']) + properties = {p['name']: p['value'] for p in image['properties'] + if not p['deleted']} image_dict = _fetch_attrs(image, glance.db.IMAGE_ATTRS) image_dict['properties'] = properties diff --git a/glance/registry/api/v1/members.py b/glance/registry/api/v1/members.py index ca001062fd..3db07d42ea 100644 --- a/glance/registry/api/v1/members.py +++ b/glance/registry/api/v1/members.py @@ -356,8 +356,7 @@ def make_member_list(members, **attr_map): """ def _fetch_memb(memb, attr_map): - return dict([(k, memb[v]) - for k, v in attr_map.items() if v in memb.keys()]) + return {k: memb[v] for k, v in attr_map.items() if v in memb.keys()} # Return the list of members with the given attribute mapping return [_fetch_memb(memb, attr_map) for memb in members] diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 36d454d004..797f0a5222 100644 --- a/glance/tests/functional/db/base.py +++ b/glance/tests/functional/db/base.py @@ -287,7 +287,7 @@ class DriverTests(object): fixture = {'properties': {'ping': 'pong'}} image = self.db_api.image_update(self.adm_context, UUID1, fixture) expected = {'ping': 'pong', 'foo': 'bar', 'far': 'boo'} - actual = dict((p['name'], p['value']) for p in image['properties']) + actual = {p['name']: p['value'] for p in image['properties']} self.assertEqual(expected, actual) self.assertNotEqual(image['created_at'], image['updated_at']) @@ -295,7 +295,7 @@ class DriverTests(object): fixture = {'properties': {'ping': 'pong'}} image = self.db_api.image_update(self.adm_context, UUID1, fixture, purge_props=True) - properties = dict((p['name'], p) for p in image['properties']) + properties = {p['name']: p for p in image['properties']} # New properties are set self.assertTrue('ping' in properties) diff --git a/glance/tests/test_hacking.py b/glance/tests/test_hacking.py index 685185d710..dd921958e1 100644 --- a/glance/tests/test_hacking.py +++ b/glance/tests/test_hacking.py @@ -72,3 +72,31 @@ class HackingTestCase(utils.BaseTestCase): self.assertEqual(0, len(list(checks.check_no_contextlib_nested( "with foo as bar")))) + + 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/glance/tests/unit/test_migrations.py b/glance/tests/unit/test_migrations.py index c59efe4a47..a1cfe84547 100644 --- a/glance/tests/unit/test_migrations.py +++ b/glance/tests/unit/test_migrations.py @@ -276,7 +276,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): return initial_values def _check_010(self, engine, data): - values = dict((c, u) for c, u in data) + values = {c: u for c, u in data} images = db_utils.get_table(engine, 'images') for row in images.select().execute(): @@ -646,7 +646,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): def _check_019(self, engine, data): image_locations = db_utils.get_table(engine, 'image_locations') records = image_locations.select().execute().fetchall() - locations = dict([(il.image_id, il.value) for il in records]) + locations = {il.image_id: il.value for il in records} self.assertEqual('http://glance.example.com', locations.get('fake-19-1')) @@ -937,7 +937,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): records = tasks_table.select().execute().fetchall() self.assertEqual(2, len(records)) - tasks = dict([(t.id, t) for t in records]) + tasks = {t.id: t for t in records} task_1 = tasks.get('task-1') self.assertEqual('some input', task_1.input) diff --git a/tools/migrate_image_owners.py b/tools/migrate_image_owners.py index 62fd9842f0..55da57d999 100644 --- a/tools/migrate_image_owners.py +++ b/tools/migrate_image_owners.py @@ -40,7 +40,7 @@ def get_owner_map(ksclient, owner_is_tenant=True): else: entities = ksclient.users.list() # build mapping of (user or tenant) name to id - return dict([(entity.name, entity.id) for entity in entities]) + return {entity.name: entity.id for entity in entities} def build_image_owner_map(owner_map, db, context):