From 091f821a6a18dce2d34370fa45a0681140a48c03 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 10 Nov 2014 13:44:36 -0500 Subject: [PATCH] - Rename get_repo_image to get_repo_image_extended and get_repo_image_directly to get_repo_image - Remove the configure call from CloseForLongOperation - Other small fixes --- data/database.py | 3 ++- data/model/legacy.py | 46 +++++++++++++++++++++--------------------- endpoints/api/image.py | 4 ++-- endpoints/registry.py | 20 +++++++++--------- endpoints/verbs.py | 2 +- tools/auditancestry.py | 4 ++-- 6 files changed, 40 insertions(+), 39 deletions(-) diff --git a/data/database.py b/data/database.py index c466a6d3f..bce76c1bb 100644 --- a/data/database.py +++ b/data/database.py @@ -48,7 +48,8 @@ class CloseForLongOperation(object): close_db_filter(None) def __exit__(self, type, value, traceback): - configure(self.config_object) + # Note: Nothing to do. The next SQL call will reconnect automatically. + pass class UseThenDisconnect(object): diff --git a/data/model/legacy.py b/data/model/legacy.py index e5b49bc53..e797b96b6 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -1034,18 +1034,18 @@ def get_repository(namespace_name, repository_name): return None -def get_repo_image_directly(namespace_name, repository_name, docker_image_id): +def get_repo_image(namespace_name, repository_name, docker_image_id): def limit_to_image_id(query): return query.where(Image.docker_image_id == docker_image_id).limit(1) - query = _get_repository_images_directly(namespace_name, repository_name, limit_to_image_id) + query = _get_repository_images(namespace_name, repository_name, limit_to_image_id) try: return query.get() except Image.DoesNotExist: return None -def get_repo_image(namespace_name, repository_name, docker_image_id): +def get_repo_image_extended(namespace_name, repository_name, docker_image_id): def limit_to_image_id(query): return query.where(Image.docker_image_id == docker_image_id).limit(1) @@ -1156,7 +1156,8 @@ def __translate_ancestry(old_ancestry, translations, repository, username, prefe # Select all the ancestor Docker IDs in a single query. old_ids = [int(id_str) for id_str in old_ancestry.split('/')[1:-1]] - old_images = {i.id: i.docker_image_id for i in Image.select().where(Image.id << old_ids)} + query = Image.select(Image.id, Image.docker_image_id).where(Image.id << old_ids) + old_images = {i.id: i.docker_image_id for i in query} # Translate the old images into new ones. new_ids = [str(translate_id(old_id, old_images[old_id])) for old_id in old_ids] @@ -1176,8 +1177,8 @@ def _find_or_link_image(existing_image, repository, username, translations, pref # it instead be done under a set of transactions? with config.app_config['DB_TRANSACTION_FACTORY'](db): # Check for an existing image, under the transaction, to make sure it doesn't already exist. - repo_image = get_repo_image_directly(repository.namespace_user.username, repository.name, - existing_image.docker_image_id) + repo_image = get_repo_image(repository.namespace_user.username, repository.name, + existing_image.docker_image_id) if repo_image: return repo_image @@ -1210,8 +1211,8 @@ def find_create_or_link_image(docker_image_id, repository, username, translation preferred_location): # First check for the image existing in the repository. If found, we simply return it. - repo_image = get_repo_image_directly(repository.namespace_user.username, repository.name, - docker_image_id) + repo_image = get_repo_image(repository.namespace_user.username, repository.name, + docker_image_id) if repo_image: return repo_image @@ -1250,8 +1251,8 @@ def find_create_or_link_image(docker_image_id, repository, username, translation # Otherwise, create a new storage directly. with config.app_config['DB_TRANSACTION_FACTORY'](db): # Final check for an existing image, under the transaction. - repo_image = get_repo_image_directly(repository.namespace_user.username, repository.name, - docker_image_id) + repo_image = get_repo_image(repository.namespace_user.username, repository.name, + docker_image_id) if repo_image: return repo_image @@ -1374,7 +1375,7 @@ def set_image_metadata(docker_image_id, namespace_name, repository_name, created fetched.storage.save() return fetched -def _get_repository_images_directly(namespace_name, repository_name, query_modifier): +def _get_repository_images(namespace_name, repository_name, query_modifier): query = (Image .select() .join(Repository) @@ -1420,12 +1421,12 @@ def _get_repository_images_base(namespace_name, repository_name, query_modifier) def lookup_repository_images(namespace_name, repository_name, docker_image_ids): - repo = get_repository(namespace_name, repository_name) - if repo is None: - return [] - - return Image.select().where(Image.repository == repo, Image.docker_image_id << docker_image_ids) - + return (Image + .select() + .join(Repository) + .join(Namespace, on=(Repository.namespace_user == Namespace.id)) + .where(Repository.name == repository_name, Namespace.username == namespace_name, + Image.docker_image_id << docker_image_ids)) def get_matching_repository_images(namespace_name, repository_name, docker_image_ids): def modify_query(q): @@ -1472,7 +1473,7 @@ def garbage_collect_repository(namespace_name, repository_name): referenced_anscestors = referenced_anscestors.union(set(ancestor_list)) referenced_anscestors.add(tag.image.id) - all_repo_images = _get_repository_images_directly(namespace_name, repository_name, lambda q: q) + all_repo_images = _get_repository_images(namespace_name, repository_name, lambda q: q) all_images = {int(img.id): img for img in all_repo_images} to_remove = set(all_images.keys()).difference(referenced_anscestors) @@ -1585,7 +1586,7 @@ def get_tag_image(namespace_name, repository_name, tag_name): def get_image_by_id(namespace_name, repository_name, docker_image_id): - image = get_repo_image(namespace_name, repository_name, docker_image_id) + image = get_repo_image_extended(namespace_name, repository_name, docker_image_id) if not image: raise DataModelException('Unable to find image \'%s\' for repo \'%s/%s\'' % (docker_image_id, namespace_name, repository_name)) @@ -1816,11 +1817,10 @@ def get_repository_delegate_tokens(namespace_name, repository_name): def get_repo_delegate_token(namespace_name, repository_name, code): repo_query = get_repository_delegate_tokens(namespace_name, repository_name) - found = list(repo_query.where(AccessToken.code == code).limit(1)) - if found: - return found[0] - else: + try: + return repo_query.where(AccessToken.code == code).get() + except AccessToken.DoesNotExist: raise InvalidTokenException('Unable to find token with code: %s' % code) diff --git a/endpoints/api/image.py b/endpoints/api/image.py index 3a6c62507..c952cc5d7 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -73,7 +73,7 @@ class RepositoryImage(RepositoryParamResource): @nickname('getImage') def get(self, namespace, repository, image_id): """ Get the information available for the specified image. """ - image = model.get_repo_image(namespace, repository, image_id) + image = model.get_repo_image_extended(namespace, repository, image_id) if not image: raise NotFound() @@ -94,7 +94,7 @@ class RepositoryImageChanges(RepositoryParamResource): @nickname('getImageChanges') def get(self, namespace, repository, image_id): """ Get the list of changes for the specified image. """ - image = model.get_repo_image(namespace, repository, image_id) + image = model.get_repo_image_extended(namespace, repository, image_id) if not image: raise NotFound() diff --git a/endpoints/registry.py b/endpoints/registry.py index bd107116a..77ddbec68 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -59,7 +59,7 @@ def require_completion(f): @wraps(f) def wrapper(namespace, repository, *args, **kwargs): image_id = kwargs['image_id'] - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) if image_is_uploading(repo_image): abort(400, 'Image %(image_id)s is being uploaded, retry later', issue='upload-in-progress', image_id=kwargs['image_id']) @@ -103,7 +103,7 @@ def head_image_layer(namespace, repository, image_id, headers): profile.debug('Checking repo permissions') if permission.can() or model.repository_is_public(namespace, repository): profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) if not repo_image: profile.debug('Image not found') abort(404, 'Image %(image_id)s not found', issue='unknown-image', @@ -136,7 +136,7 @@ def get_image_layer(namespace, repository, image_id, headers): profile.debug('Checking repo permissions') if permission.can() or model.repository_is_public(namespace, repository): profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) profile.debug('Looking up the layer path') try: @@ -174,7 +174,7 @@ def put_image_layer(namespace, repository, image_id): abort(403) profile.debug('Retrieving image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) try: profile.debug('Retrieving image data') uuid = repo_image.storage.uuid @@ -298,7 +298,7 @@ def put_image_checksum(namespace, repository, image_id): issue='missing-checksum-cookie', image_id=image_id) profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) if not repo_image or not repo_image.storage: abort(404, 'Image not found: %(image_id)s', issue='unknown-image', image_id=image_id) @@ -353,7 +353,7 @@ def get_image_json(namespace, repository, image_id, headers): abort(403) profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) profile.debug('Looking up repo layer data') try: @@ -384,7 +384,7 @@ def get_image_ancestry(namespace, repository, image_id, headers): abort(403) profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) profile.debug('Looking up image data') try: @@ -448,7 +448,7 @@ def put_image_json(namespace, repository, image_id): issue='invalid-request', image_id=image_id) profile.debug('Looking up repo image') - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) if not repo_image: profile.debug('Image not found') abort(404, 'Image %(image_id)s not found', issue='unknown-image', @@ -465,7 +465,7 @@ def put_image_json(namespace, repository, image_id): parent_image = None if parent_id: profile.debug('Looking up parent image') - parent_image = model.get_repo_image(namespace, repository, parent_id) + parent_image = model.get_repo_image_extended(namespace, repository, parent_id) parent_uuid = parent_image and parent_image.storage.uuid parent_locations = parent_image and parent_image.storage.locations @@ -518,7 +518,7 @@ def put_image_json(namespace, repository, image_id): def process_image_changes(namespace, repository, image_id): logger.debug('Generating diffs for image: %s' % image_id) - repo_image = model.get_repo_image(namespace, repository, image_id) + repo_image = model.get_repo_image_extended(namespace, repository, image_id) if not repo_image: logger.warning('No image for id: %s', image_id) return None, None diff --git a/endpoints/verbs.py b/endpoints/verbs.py index 92f3fa24d..33a214788 100644 --- a/endpoints/verbs.py +++ b/endpoints/verbs.py @@ -83,7 +83,7 @@ def get_squashed_tag(namespace, repository, tag): abort(404) # Lookup the tag's image and storage. - repo_image = model.get_repo_image(namespace, repository, tag_image.docker_image_id) + repo_image = model.get_repo_image_extended(namespace, repository, tag_image.docker_image_id) if not repo_image: abort(404) diff --git a/tools/auditancestry.py b/tools/auditancestry.py index 59d636836..27fd11d8c 100644 --- a/tools/auditancestry.py +++ b/tools/auditancestry.py @@ -27,7 +27,7 @@ bad_count = 0 good_count = 0 def resolve_or_create(repo, docker_image_id, new_ancestry): - existing = model.get_repo_image(repo.namespace_user.username, repo.name, docker_image_id) + existing = model.get_repo_image_extended(repo.namespace_user.username, repo.name, docker_image_id) if existing: logger.debug('Found existing image: %s, %s', existing.id, docker_image_id) return existing @@ -63,7 +63,7 @@ def all_ancestors_exist(ancestors): cant_fix = [] for img in query: try: - with_locations = model.get_repo_image(img.repository.namespace_user.username, + with_locations = model.get_repo_image_extended(img.repository.namespace_user.username, img.repository.name, img.docker_image_id) ancestry_storage = store.image_ancestry_path(img.storage.uuid) if store.exists(with_locations.storage.locations, ancestry_storage):