diff --git a/buildman/component/buildcomponent.py b/buildman/component/buildcomponent.py index 4b25c4636..d0e9edfae 100644 --- a/buildman/component/buildcomponent.py +++ b/buildman/component/buildcomponent.py @@ -17,9 +17,10 @@ from buildman.jobutil.buildstatus import StatusHandler from buildman.jobutil.workererror import WorkerError from app import app -from data import model from data.database import BUILD_PHASE, UseThenDisconnect from data.model import InvalidRepositoryBuildException +from data.registry_model import registry_model +from data.registry_model.datatypes import RepositoryReference from util import slash_join HEARTBEAT_DELTA = datetime.timedelta(seconds=60) @@ -29,6 +30,9 @@ INITIAL_TIMEOUT = 25 SUPPORTED_WORKER_VERSIONS = ['0.3'] +# Label which marks a manifest with its source build ID. +INTERNAL_LABEL_BUILD_UUID = 'quay.build.uuid' + logger = logging.getLogger(__name__) class ComponentStatus(object): @@ -357,18 +361,17 @@ class BuildComponent(BaseComponent): # Label the pushed manifests with the build metadata. manifest_digests = kwargs.get('digests') or [] - for digest in manifest_digests: - with UseThenDisconnect(app.config): - try: - manifest = model.tag.load_manifest_by_digest(self._current_job.namespace, - self._current_job.repo_name, digest) - model.label.create_manifest_label(manifest, model.label.INTERNAL_LABEL_BUILD_UUID, - build_id, 'internal', 'text/plain') - except model.InvalidManifestException: - logger.debug('Could not find built manifest with digest %s under repo %s/%s for build %s', - digest, self._current_job.namespace, self._current_job.repo_name, - build_id) - continue + repository = registry_model.lookup_repository(self._current_job.namespace, + self._current_job.repo_name) + if repository is not None: + for digest in manifest_digests: + with UseThenDisconnect(app.config): + manifest = registry_model.lookup_manifest_by_digest(repository, digest) + if manifest is None: + continue + + registry_model.create_manifest_label(manifest, INTERNAL_LABEL_BUILD_UUID, + build_id, 'internal', 'text/plain') # Send the notification that the build has completed successfully. self._current_job.send_notification('build_success', diff --git a/data/model/label.py b/data/model/label.py index 1592efaa6..3e461e2d7 100644 --- a/data/model/label.py +++ b/data/model/label.py @@ -11,9 +11,6 @@ from util.validation import is_json logger = logging.getLogger(__name__) -# Label which marks a manifest with its source build ID. -INTERNAL_LABEL_BUILD_UUID = 'quay.build.uuid' - @lru_cache(maxsize=1) def get_label_source_types(): diff --git a/data/registry_model/datatype.py b/data/registry_model/datatype.py new file mode 100644 index 000000000..0b72b5fa8 --- /dev/null +++ b/data/registry_model/datatype.py @@ -0,0 +1,49 @@ +# pylint: disable=protected-access + +from functools import wraps, total_ordering + +def datatype(name, static_fields): + """ Defines a base class for a datatype that will represent a row from the database, + in an abstracted form. + """ + @total_ordering + class DataType(object): + __name__ = name + + def __init__(self, **kwargs): + self._db_id = kwargs.pop('db_id', None) + self._inputs = kwargs.pop('inputs', None) + self._fields = kwargs + + for name in static_fields: + assert name in self._fields, 'Missing field %s' % name + + def __eq__(self, other): + return self._db_id == other._db_id + + def __lt__(self, other): + return self._db_id < other._db_id + + def __getattr__(self, name): + if name in static_fields: + return self._fields[name] + + raise AttributeError('Unknown field `%s`' % name) + + return DataType + + +def requiresinput(input_name): + """ Marks a property on the data type as requiring an input to be invoked. """ + def inner(func): + @wraps(func) + def wrapper(self, *args, **kwargs): + if self._inputs.get(input_name) is None: + raise Exception('Cannot invoke function with missing input `%s`', input_name) + + kwargs[input_name] = self._inputs[input_name] + result = func(self, *args, **kwargs) + return result + + return wrapper + return inner diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index 3f2cae187..66c39c2f4 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -1,20 +1,82 @@ -from collections import namedtuple +from data.registry_model.datatype import datatype, requiresinput -class RepositoryReference(object): +class RepositoryReference(datatype('Repository', [])): """ RepositoryReference is a reference to a repository, passed to registry interface methods. """ - def __init__(self, repo_id): - self.repo_id = repo_id - @classmethod def for_repo_obj(cls, repo_obj): - return RepositoryReference(repo_obj.id) + if repo_obj is None: + return None + + return RepositoryReference(db_id=repo_obj.id) -class Tag(namedtuple('Tag', ['id', 'name'])): +class Label(datatype('Label', ['key', 'value'])): + """ Label represents a label on a manifest. """ + @classmethod + def for_label(cls, label): + if label is None: + return None + + return Label(db_id=label.id, key=label.key, value=label.value) + + +class Tag(datatype('Tag', ['name'])): """ Tag represents a tag in a repository, which points to a manifest or image. """ @classmethod def for_repository_tag(cls, repository_tag): if repository_tag is None: return None - return Tag(id=repository_tag.id, name=repository_tag.name) + return Tag(db_id=repository_tag.id, name=repository_tag.name) + + +class Manifest(datatype('Manifest', ['digest'])): + """ Manifest represents a manifest in a repository. """ + @classmethod + def for_tag_manifest(cls, tag_manifest): + if tag_manifest is None: + return None + + return Manifest(db_id=tag_manifest.id, digest=tag_manifest.digest) + + +class LegacyImage(datatype('LegacyImage', ['docker_image_id', 'created', 'comment', 'command', + 'image_size', 'uploading'])): + """ LegacyImage represents a Docker V1-style image found in a repository. """ + @classmethod + def for_image(cls, image, images_map=None, tags_map=None): + if image is None: + return None + + return LegacyImage(db_id=image.id, + inputs=dict(images_map=images_map, tags_map=tags_map, + ancestor_id_list=image.ancestor_id_list()), + docker_image_id=image.docker_image_id, + created=image.created, + comment=image.comment, + command=image.command, + image_size=image.storage.image_size, + uploading=image.storage.uploading) + + @property + @requiresinput('images_map') + @requiresinput('ancestor_id_list') + def parents(self, images_map, ancestor_id_list): + """ Returns the parent images for this image. Raises an exception if the parents have + not been loaded before this property is invoked. + """ + return [LegacyImage.for_image(images_map[ancestor_id], images_map=images_map) + for ancestor_id in ancestor_id_list + if images_map.get(ancestor_id)] + + @property + @requiresinput('tags_map') + def tags(self, tags_map): + """ Returns the tags pointing to this image. Raises an exception if the tags have + not been loaded before this property is invoked. + """ + tags = tags_map.get(self._db_id) + if not tags: + return [] + + return [Tag.for_repository_tag(tag) for tag in tags] diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index e67366733..9961f7dda 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -19,3 +19,34 @@ class RegistryDataInterface(object): """ Returns the most recently pushed alive tag in the repository, if any. If none, returns None. """ + + @abstractmethod + def lookup_repository(self, namespace_name, repo_name, kind_filter=None): + """ Looks up and returns a reference to the repository with the given namespace and name, + or None if none. """ + + @abstractmethod + def get_manifest_for_tag(self, tag): + """ Returns the manifest associated with the given tag. """ + + @abstractmethod + def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=False): + """ Looks up the manifest with the given digest under the given repository and returns it + or None if none. """ + + @abstractmethod + def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None): + """ Creates a label on the manifest with the given key and value. """ + + @abstractmethod + def get_legacy_images(self, repository_ref): + """ + Returns an iterator of all the LegacyImage's defined in the matching repository. + """ + + @abstractmethod + def get_legacy_image(self, repository_ref, docker_image_id, include_parents=False): + """ + Returns the matching LegacyImages under the matching repository, if any. If none, + returns None. + """ diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py index 5cd6ed631..b879f96f0 100644 --- a/data/registry_model/registry_pre_oci_model.py +++ b/data/registry_model/registry_pre_oci_model.py @@ -1,6 +1,11 @@ +# pylint: disable=protected-access + +from collections import defaultdict + +from data import database from data import model from data.registry_model.interface import RegistryDataInterface -from data.registry_model.datatypes import Tag +from data.registry_model.datatypes import Tag, RepositoryReference, Manifest, LegacyImage, Label class PreOCIModel(RegistryDataInterface): @@ -13,15 +18,92 @@ class PreOCIModel(RegistryDataInterface): """ Finds an alive tag in the repository matching one of the given tag names and returns it or None if none. """ - found_tag = model.tag.find_matching_tag(repository_ref.repo_id, tag_names) + found_tag = model.tag.find_matching_tag(repository_ref._db_id, tag_names) return Tag.for_repository_tag(found_tag) def get_most_recent_tag(self, repository_ref): """ Returns the most recently pushed alive tag in the repository, if any. If none, returns None. """ - found_tag = model.tag.get_most_recent_tag(repository_ref.repo_id) + found_tag = model.tag.get_most_recent_tag(repository_ref._db_id) return Tag.for_repository_tag(found_tag) + def lookup_repository(self, namespace_name, repo_name, kind_filter=None): + """ Looks up and returns a reference to the repository with the given namespace and name, + or None if none. """ + repo = model.repository.get_repository(namespace_name, repo_name, kind_filter=kind_filter) + return RepositoryReference.for_repo_obj(repo) + + def get_manifest_for_tag(self, tag): + """ Returns the manifest associated with the given tag. """ + try: + tag_manifest = database.TagManifest.get(tag_id=tag._db_id) + except database.TagManifest.DoesNotExist: + return + + return Manifest.for_tag_manifest(tag_manifest) + + def lookup_manifest_by_digest(self, repository_ref, manifest_digest, allow_dead=False): + """ Looks up the manifest with the given digest under the given repository and returns it + or None if none. """ + repo = model.repository.lookup_repository(repository_ref._db_id) + if repo is None: + return None + + tag_manifest = model.tag.load_manifest_by_digest(repo.namespace_user.username, + repo.name, + manifest_digest, allow_dead=allow_dead) + return Manifest.for_tag_manifest(tag_manifest) + + def create_manifest_label(self, manifest, key, value, source_type_name, media_type_name=None): + """ Creates a label on the manifest with the given key and value. """ + try: + tag_manifest = database.TagManifest.get(id=manifest._db_id) + except database.TagManifest.DoesNotExist: + return None + + label = model.label.create_manifest_label(tag_manifest, key, value, source_type_name, + media_type_name) + return Label.for_label(label) + + def get_legacy_images(self, repository_ref): + """ + Returns an iterator of all the LegacyImage's defined in the matching repository. + """ + repo = model.repository.lookup_repository(repository_ref._db_id) + if repo is None: + return None + + all_images = model.image.get_repository_images_without_placements(repo) + all_images_map = {image.id: image for image in all_images} + + all_tags = model.tag.list_repository_tags(repo.namespace_user.username, repo.name) + tags_by_image_id = defaultdict(list) + for tag in all_tags: + tags_by_image_id[tag.image_id].append(tag) + + return [LegacyImage.for_image(image, images_map=all_images_map, tags_map=tags_by_image_id) + for image in all_images] + + def get_legacy_image(self, repository_ref, docker_image_id, include_parents=False): + """ + Returns the matching LegacyImages under the matching repository, if any. If none, + returns None. + """ + repo = model.repository.lookup_repository(repository_ref._db_id) + if repo is None: + return None + + image = model.image.get_image(repository_ref._db_id, docker_image_id) + if image is None: + return None + + parent_images_map = None + if include_parents: + parent_images = model.image.get_parent_images(repo.namespace_user.username, repo.name, image) + parent_images_map = {image.id: image for image in parent_images} + + return LegacyImage.for_image(image, images_map=parent_images_map) + pre_oci_model = PreOCIModel() diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py index 6e81c3fff..d3047cff1 100644 --- a/data/registry_model/test/test_pre_oci_model.py +++ b/data/registry_model/test/test_pre_oci_model.py @@ -39,3 +39,80 @@ def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model) assert found is None else: assert found.name in expected + + +@pytest.mark.parametrize('repo_namespace, repo_name, expected', [ + ('devtable', 'simple', True), + ('buynlarge', 'orgrepo', True), + ('buynlarge', 'unknownrepo', False), +]) +def test_lookup_repository(repo_namespace, repo_name, expected, pre_oci_model): + repo_ref = pre_oci_model.lookup_repository(repo_namespace, repo_name) + if expected: + assert repo_ref + else: + assert repo_ref is None + + +@pytest.mark.parametrize('repo_namespace, repo_name', [ + ('devtable', 'simple'), + ('buynlarge', 'orgrepo'), +]) +def test_lookup_manifests(repo_namespace, repo_name, pre_oci_model): + repo = model.repository.get_repository(repo_namespace, repo_name) + repository_ref = RepositoryReference.for_repo_obj(repo) + found_tag = pre_oci_model.find_matching_tag(repository_ref, ['latest']) + found_manifest = pre_oci_model.get_manifest_for_tag(found_tag) + found = pre_oci_model.lookup_manifest_by_digest(repository_ref, found_manifest.digest) + assert found._db_id == found_manifest._db_id + assert found.digest == found_manifest.digest + + +def test_create_manifest_label(pre_oci_model): + repo = model.repository.get_repository('devtable', 'simple') + repository_ref = RepositoryReference.for_repo_obj(repo) + found_tag = pre_oci_model.find_matching_tag(repository_ref, ['latest']) + found_manifest = pre_oci_model.get_manifest_for_tag(found_tag) + + pre_oci_model.create_manifest_label(found_manifest, 'foo', 'bar', 'internal') + + +@pytest.mark.parametrize('repo_namespace, repo_name', [ + ('devtable', 'simple'), + ('devtable', 'complex'), + ('devtable', 'history'), + ('buynlarge', 'orgrepo'), +]) +def test_legacy_images(repo_namespace, repo_name, pre_oci_model): + repository_ref = pre_oci_model.lookup_repository(repo_namespace, repo_name) + legacy_images = pre_oci_model.get_legacy_images(repository_ref) + assert len(legacy_images) + + found_tags = set() + for image in legacy_images: + found_image = pre_oci_model.get_legacy_image(repository_ref, image.docker_image_id, + include_parents=True) + + assert found_image.docker_image_id == image.docker_image_id + assert found_image.parents == image.parents + + # Check that the tags list can be retrieved. + assert image.tags is not None + found_tags.update({tag.name for tag in image.tags}) + + # Check against the actual DB row. + model_image = model.image.get_image(repository_ref._db_id, found_image.docker_image_id) + assert model_image.id == found_image._db_id + assert ([pid for pid in model_image.ancestor_id_list()] == + [p._db_id for p in found_image.parents]) + + # Try without parents and ensure it raises an exception. + found_image = pre_oci_model.get_legacy_image(repository_ref, image.docker_image_id, + include_parents=False) + with pytest.raises(Exception): + assert not found_image.parents + + assert found_tags + + unknown = pre_oci_model.get_legacy_image(repository_ref, 'unknown', include_parents=True) + assert unknown is None diff --git a/endpoints/api/image.py b/endpoints/api/image.py index 522fdf951..53c4bf68b 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -1,11 +1,36 @@ """ List and lookup repository images. """ +import json +from data.registry_model import registry_model from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource, - path_param, disallow_for_app_repositories) -from endpoints.api.image_models_pre_oci import pre_oci_model as model + path_param, disallow_for_app_repositories, format_date) from endpoints.exception import NotFound +def image_dict(image, with_history=False, with_tags=False): + image_data = { + 'id': image.docker_image_id, + 'created': format_date(image.created), + 'comment': image.comment, + 'command': json.loads(image.command) if image.command else None, + 'size': image.image_size, + 'uploading': image.uploading, + 'sort_index': len(image.parents), + } + + if with_tags: + image_data['tags'] = [tag.name for tag in image.tags] + + if with_history: + image_data['history'] = [image_dict(parent, with_history, with_tags) + for parent in image.parents] + + # Calculate the ancestors string, with the DBID's replaced with the docker IDs. + parent_docker_ids = [parent_image.docker_image_id for parent_image in image.parents] + image_data['ancestors'] = '/{0}/'.format('/'.join(parent_docker_ids)) + return image_data + + @resource('/v1/repository//image/') @path_param('repository', 'The full path of the repository. e.g. namespace/name') class RepositoryImageList(RepositoryParamResource): @@ -16,11 +41,12 @@ class RepositoryImageList(RepositoryParamResource): @disallow_for_app_repositories def get(self, namespace, repository): """ List the images for the specified repository. """ - images = model.get_repository_images(namespace, repository) - if images is None: + repo_ref = registry_model.lookup_repository(namespace, repository) + if repo_ref is None: raise NotFound() - return {'images': [image.to_dict() for image in images]} + images = registry_model.get_legacy_images(repo_ref) + return {'images': [image_dict(image, with_tags=True) for image in images]} @resource('/v1/repository//image/') @@ -34,8 +60,12 @@ class RepositoryImage(RepositoryParamResource): @disallow_for_app_repositories def get(self, namespace, repository, image_id): """ Get the information available for the specified image. """ - image = model.get_repository_image(namespace, repository, image_id) + repo_ref = registry_model.lookup_repository(namespace, repository) + if repo_ref is None: + raise NotFound() + + image = registry_model.get_legacy_image(repo_ref, image_id, include_parents=True) if image is None: raise NotFound() - return image.to_dict() + return image_dict(image, with_history=True) diff --git a/endpoints/api/image_models_interface.py b/endpoints/api/image_models_interface.py deleted file mode 100644 index 0cd5c0a8f..000000000 --- a/endpoints/api/image_models_interface.py +++ /dev/null @@ -1,77 +0,0 @@ -import json - -from endpoints.api import format_date - -from abc import ABCMeta, abstractmethod -from collections import namedtuple -from six import add_metaclass - -class Image(namedtuple('Image', ['docker_image_id', 'created', 'comment', 'command', 'image_size', - 'uploading', 'parents'])): - """ - Image represents an image. - :type name: string - """ - - def to_dict(self): - image_data = { - 'id': self.docker_image_id, - 'created': format_date(self.created), - 'comment': self.comment, - 'command': json.loads(self.command) if self.command else None, - 'size': self.image_size, - 'uploading': self.uploading, - 'sort_index': len(self.parents), - } - - # Calculate the ancestors string, with the DBID's replaced with the docker IDs. - parent_docker_ids = [parent_image.docker_image_id for parent_image in self.parents] - image_data['ancestors'] = '/{0}/'.format('/'.join(parent_docker_ids)) - - return image_data - -class ImageWithTags(namedtuple('ImageWithTags', ['image', 'tag_names'])): - """ - ImageWithTags represents an image, along with the tags that point to it. - :type image: Image - :type tag_names: list of string - """ - - def to_dict(self): - image_dict = self.image.to_dict() - image_dict['tags'] = self.tag_names - return image_dict - - -class ImageWithHistory(namedtuple('ImageWithHistory', ['image'])): - """ - ImageWithHistory represents an image, along with its full parent image dictionaries. - :type image: Image - :type history: list of Image parents (name is old and must be kept for compat) - """ - - def to_dict(self): - image_dict = self.image.to_dict() - image_dict['history'] = [parent_image.to_dict() for parent_image in self.image.parents] - return image_dict - - -@add_metaclass(ABCMeta) -class ImageInterface(object): - """ - Interface that represents all data store interactions required by the image API endpoint. - """ - - @abstractmethod - def get_repository_images(self, namespace_name, repo_name): - """ - Returns an iterator of all the ImageWithTag's defined in the matching repository. If the - repository doesn't exist, returns None. - """ - - @abstractmethod - def get_repository_image(self, namespace_name, repo_name, docker_image_id): - """ - Returns the matching ImageWithHistory under the matching repository, if any. If none, - returns None. - """ diff --git a/endpoints/api/image_models_pre_oci.py b/endpoints/api/image_models_pre_oci.py deleted file mode 100644 index e29c294b0..000000000 --- a/endpoints/api/image_models_pre_oci.py +++ /dev/null @@ -1,56 +0,0 @@ -from collections import defaultdict -from data import model -from endpoints.api.image_models_interface import (ImageInterface, ImageWithHistory, ImageWithTags, - Image) - -def _image(namespace_name, repo_name, image, all_images, include_parents=True): - parent_image_tuples = [] - if include_parents: - parent_images = [all_images[ancestor_id] for ancestor_id in image.ancestor_id_list() - if all_images.get(ancestor_id)] - parent_image_tuples = [_image(namespace_name, repo_name, parent_image, all_images, False) - for parent_image in parent_images] - - return Image(image.docker_image_id, image.created, image.comment, image.command, - image.storage.image_size, image.storage.uploading, parent_image_tuples) - -def _tag_names(image, tags_by_docker_id): - return [tag.name for tag in tags_by_docker_id.get(image.docker_image_id, [])] - - -class PreOCIModel(ImageInterface): - """ - PreOCIModel implements the data model for the Image API using a database schema - before it was changed to support the OCI specification. - """ - def get_repository_images(self, namespace_name, repo_name): - repo = model.repository.get_repository(namespace_name, repo_name) - if not repo: - return None - - all_images = model.image.get_repository_images_without_placements(repo) - all_images_map = {image.id: image for image in all_images} - - all_tags = list(model.tag.list_repository_tags(namespace_name, repo_name)) - - tags_by_docker_id = defaultdict(list) - for tag in all_tags: - tags_by_docker_id[tag.image.docker_image_id].append(tag) - - def _build_image(image): - image_itself = _image(namespace_name, repo_name, image, all_images_map) - return ImageWithTags(image_itself, tag_names=_tag_names(image, tags_by_docker_id)) - - return [_build_image(image) for image in all_images] - - def get_repository_image(self, namespace_name, repo_name, docker_image_id): - image = model.image.get_repo_image_and_storage(namespace_name, repo_name, docker_image_id) - if not image: - return None - - parent_images = model.image.get_parent_images(namespace_name, repo_name, image) - all_images_map = {image.id: image for image in parent_images} - return ImageWithHistory(_image(namespace_name, repo_name, image, all_images_map)) - - -pre_oci_model = PreOCIModel() diff --git a/endpoints/api/manifest_models_interface.py b/endpoints/api/manifest_models_interface.py index 03615b208..539399b2b 100644 --- a/endpoints/api/manifest_models_interface.py +++ b/endpoints/api/manifest_models_interface.py @@ -1,6 +1,7 @@ from abc import ABCMeta, abstractmethod from collections import namedtuple +from endpoints.api.image import image_dict from six import add_metaclass @@ -40,7 +41,7 @@ class ManifestAndImage( return { 'digest': self.digest, 'manifest_data': self.manifest_data, - 'image': self.image.to_dict(), + 'image': image_dict(self.image), } diff --git a/endpoints/api/manifest_models_pre_oci.py b/endpoints/api/manifest_models_pre_oci.py index a5d3d78af..6a40eb76a 100644 --- a/endpoints/api/manifest_models_pre_oci.py +++ b/endpoints/api/manifest_models_pre_oci.py @@ -2,7 +2,7 @@ import json from manifest_models_interface import ManifestLabel, ManifestLabelInterface, ManifestAndImage from data import model -from image_models_pre_oci import pre_oci_model as image_models +from data.registry_model import registry_model class ManifestLabelPreOCI(ManifestLabelInterface): @@ -47,8 +47,14 @@ class ManifestLabelPreOCI(ManifestLabelInterface): return None # TODO: remove this dependency on image once we've moved to the new data model. - image = image_models.get_repository_image(namespace_name, repository_name, - tag_manifest.tag.image.docker_image_id) + repo_ref = registry_model.lookup_repository(namespace_name, repository_name) + if repo_ref is None: + return None + + image = registry_model.get_legacy_image(repo_ref, tag_manifest.tag.image.docker_image_id, + include_parents=True) + if image is None: + return None manifest_data = json.loads(tag_manifest.json_data) return ManifestAndImage(digest=digest, manifest_data=manifest_data, image=image)