From ab2f04433143f0cbb06b56db78ebd5820d47d792 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 13 Apr 2017 16:34:14 -0400 Subject: [PATCH] Switch get repo API to use a single list tags query Should make things faster since the join occurs on the database side --- data/model/tag.py | 27 ++++++++++++++++++++++----- endpoints/api/repository.py | 14 ++++++-------- endpoints/api/tag.py | 2 +- test/test_api_usage.py | 20 +++++++++++++++++++- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/data/model/tag.py b/data/model/tag.py index 0fc231f53..650b41589 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -83,13 +83,30 @@ def filter_tags_have_repository_event(query, event): .order_by(RepositoryTag.lifetime_start_ts.desc())) -def get_tag_manifests(tags): - """ Returns a map from tag ID to its associated manifest, if any. """ +def get_tag_manifest_digests(tags): + """ Returns a map from tag ID to its associated manifest digest, if any. """ if not tags: return dict() - manifests = TagManifest.select().where(TagManifest.tag << [t.id for t in tags]) - return {manifest.tag_id:manifest for manifest in manifests} + manifests = (TagManifest + .select(TagManifest.tag, TagManifest.digest) + .where(TagManifest.tag << [t.id for t in tags])) + + return {manifest.tag_id: manifest.digest for manifest in manifests} + + +def list_active_repo_tags(repo): + """ Returns all of the active, non-hidden tags in a repository, joined to they images + and (if present), their manifest. + """ + query = _tag_alive(RepositoryTag + .select(RepositoryTag, Image, TagManifest.digest) + .join(Image) + .where(RepositoryTag.repository == repo) + .switch(RepositoryTag) + .join(TagManifest, JOIN_LEFT_OUTER)) + + return query def list_repository_tags(namespace_name, repository_name, include_hidden=False, @@ -320,7 +337,7 @@ def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None): if not tags: return [], {}, False - manifest_map = get_tag_manifests(tags) + manifest_map = get_tag_manifest_digests(tags) return tags[0:size], manifest_map, len(tags) > size diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 33f7f3eb6..86f8fe4d4 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -322,7 +322,7 @@ class Repository(RepositoryParamResource): return repo_data # Older image-only repo code. - def tag_view(tag, manifest): + def tag_view(tag): tag_info = { 'name': tag.name, 'image_id': tag.image.docker_image_id, @@ -333,16 +333,14 @@ class Repository(RepositoryParamResource): last_modified = format_date(datetime.fromtimestamp(tag.lifetime_start_ts)) tag_info['last_modified'] = last_modified - if manifest is not None: - tag_info['manifest_digest'] = manifest.digest + if tag.tagmanifest is not None: + tag_info['manifest_digest'] = tag.tagmanifest.digest return tag_info stats = None - tags = model.tag.list_repository_tags(namespace, repository, include_storage=True) - manifests = model.tag.get_tag_manifests(tags) - - tag_dict = {tag.name: tag_view(tag, manifests.get(tag.id)) for tag in tags} + tags = model.tag.list_active_repo_tags(repo) + tag_dict = {tag.name: tag_view(tag) for tag in tags} if parsed_args['includeStats']: stats = [] found_dates = {} @@ -419,7 +417,7 @@ class Repository(RepositoryParamResource): # Remove any builds from the queue. dockerfile_build_queue.delete_namespaced_items(namespace, repository) - + log_action('delete_repo', namespace, {'repo': repository, 'namespace': namespace}) return '', 204 diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index 014052ad7..33e997e07 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -44,7 +44,7 @@ class ListRepositoryTags(RepositoryParamResource): tag_info['end_ts'] = tag.lifetime_end_ts if tag.id in manifest_map: - tag_info['manifest_digest'] = manifest_map[tag.id].digest + tag_info['manifest_digest'] = manifest_map[tag.id] return tag_info diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 8afe5f5a6..c8b7230fa 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -2425,6 +2425,24 @@ class TestDeleteRepository(ApiTestCase): class TestGetRepository(ApiTestCase): PUBLIC_REPO = PUBLIC_USER + '/publicrepo' + def test_get_largerepo(self): + self.login(ADMIN_ACCESS_USER) + + # base + repo + is_starred + tags + with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 4): + self.getJsonResponse(Repository, + params=dict(repository=ADMIN_ACCESS_USER + '/simple')) + + # base + repo + is_starred + tags + with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 4): + json = self.getJsonResponse(Repository, + params=dict(repository=ADMIN_ACCESS_USER + '/gargantuan')) + + self.assertEquals(ADMIN_ACCESS_USER, json['namespace']) + self.assertEquals('gargantuan', json['name']) + + self.assertEquals(False, json['is_public']) + def test_getrepo_badnames(self): self.login(ADMIN_ACCESS_USER) @@ -4540,7 +4558,7 @@ class TestRepositoryImageSecurity(ApiTestCase): expected_code=200) - + class TestSuperUserCustomCertificates(ApiTestCase): def test_custom_certificates(self):