Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DictTuple exception when invoking images.get_all() #8441

Merged
merged 6 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions notebooks/api/0.8/10-container-images.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"\n",
"RUN pip install pydicom\n",
"\n",
"\"\"\""
"\"\"\".strip()"
]
},
{
Expand Down Expand Up @@ -205,7 +205,7 @@
"source": [
"workerimage: SyftWorkerImage = None\n",
"for image in dockerfile_list:\n",
" if image.config.dockerfile == custom_dockerfile_str:\n",
" if not image.is_prebuilt and image.config.dockerfile == custom_dockerfile_str:\n",
" workerimage = image\n",
" break\n",
"\n",
Expand Down Expand Up @@ -1051,7 +1051,7 @@
"FROM openmined/grid-backend:0.8.4-beta.12\n",
"\n",
"RUN pip install opendp\n",
"\"\"\"\n",
"\"\"\".strip()\n",
"\n",
"docker_config_2 = sy.DockerWorkerConfig(dockerfile=custom_dockerfile_str_2)"
]
Expand Down Expand Up @@ -1202,7 +1202,7 @@
"FROM openmined/grid-backend:0.8.4-beta.12\n",
"\n",
"RUN pip install recordlinkage\n",
"\"\"\"\n",
"\"\"\".strip()\n",
"\n",
"docker_config_3 = sy.DockerWorkerConfig(dockerfile=custom_dockerfile_str_3)\n",
"\n",
Expand Down Expand Up @@ -1406,7 +1406,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.2"
"version": "3.11.7"
}
},
"nbformat": 4,
Expand Down
8 changes: 4 additions & 4 deletions notebooks/api/0.8/11-container-images-k8s.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
"\n",
"RUN pip install pydicom\n",
"\n",
"\"\"\""
"\"\"\".strip()"
]
},
{
Expand Down Expand Up @@ -205,7 +205,7 @@
" (\n",
" image\n",
" for image in dockerfile_list\n",
" if image.config.dockerfile == custom_dockerfile_str\n",
" if not image.is_prebuilt and image.config.dockerfile == custom_dockerfile_str\n",
" ),\n",
" None,\n",
")\n",
Expand Down Expand Up @@ -766,7 +766,7 @@
"FROM {registry}/{repo}:{tag}\n",
"\n",
"RUN pip install opendp\n",
"\"\"\"\n",
"\"\"\".strip()\n",
"\n",
"docker_config_opendp = sy.DockerWorkerConfig(dockerfile=dockerfile_opendp)"
]
Expand Down Expand Up @@ -996,7 +996,7 @@
"FROM {registry}/{repo}:{tag}\n",
"\n",
"RUN pip install recordlinkage\n",
"\"\"\"\n",
"\"\"\".strip()\n",
"\n",
"docker_config_recordlinkage = sy.DockerWorkerConfig(dockerfile=dockerfile_recordlinkage)\n",
"\n",
Expand Down
23 changes: 23 additions & 0 deletions packages/syft/src/syft/custom_worker/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,35 @@ def get_signature(self) -> str:
return sha256(self.json(sort_keys=True).encode()).hexdigest()


@serializable()
class PrebuiltWorkerConfig(WorkerConfig):
# tag that is already built and pushed in some registry
tag: str
description: Optional[str]

def __str__(self) -> str:
if self.description:
return f"prebuilt tag='{self.tag}' description='{self.description}'"
else:
return f"prebuilt tag='{self.tag}'"

def set_description(self, description_text: str) -> None:
self.description = description_text


@serializable()
class DockerWorkerConfig(WorkerConfig):
dockerfile: str
file_name: Optional[str]
description: Optional[str]

@validator("dockerfile")
def validate_dockerfile(cls, dockerfile: str) -> str:
if not dockerfile:
raise ValueError("Dockerfile cannot be empty")
dockerfile = dockerfile.strip()
return dockerfile

@classmethod
def from_path(
cls,
Expand Down
3 changes: 3 additions & 0 deletions packages/syft/src/syft/service/worker/image_identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,8 @@ def registry_host(self) -> str:
def __hash__(self) -> int:
return hash(self.repo + self.tag + str(hash(self.registry)))

def __str__(self) -> str:
return self.full_name_with_tag

def __repr__(self) -> str:
return f"SyftWorkerImageIdentifier(repo={self.repo}, tag={self.tag}, registry={self.registry})"
10 changes: 5 additions & 5 deletions packages/syft/src/syft/service/worker/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
from ...custom_worker.builder_types import ImageBuildResult
from ...custom_worker.builder_types import ImagePushResult
from ...custom_worker.config import DockerWorkerConfig
from ...custom_worker.config import PrebuiltWorkerConfig
from ...custom_worker.k8s import PodStatus
from ...custom_worker.runner_k8s import KubernetesRunner
from ...node.credentials import SyftVerifyKey
from ...types.datetime import DateTime
from ...types.uid import UID
from ...util.util import get_queue_address
from ..response import SyftError
Expand Down Expand Up @@ -489,16 +489,16 @@ def create_default_image(
)
else:
# in k8s we don't need to build the image, just the tag of backend is enough

# a very bad and hacky way to keep the Stash's unique `config` requirment happy
worker_config = DockerWorkerConfig(dockerfile=tag)
worker_config = PrebuiltWorkerConfig(
tag=tag,
description="Prebuilt default worker image",
)

# create SyftWorkerImage from a pre-built image
_new_image = SyftWorkerImage(
config=worker_config,
created_by=credentials,
image_identifier=SyftWorkerImageIdentifier.from_str(tag),
built_at=DateTime.now(),
)

result = image_stash.get_by_docker_config(
Expand Down
17 changes: 14 additions & 3 deletions packages/syft/src/syft/service/worker/worker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Optional

# relative
from ...custom_worker.config import PrebuiltWorkerConfig
from ...custom_worker.config import WorkerConfig
from ...node.credentials import SyftVerifyKey
from ...serde.serializable import serializable
Expand All @@ -19,7 +20,13 @@ class SyftWorkerImage(SyftObject):

__attr_unique__ = ["config"]
__attr_searchable__ = ["config", "image_hash", "created_by"]
__repr_attrs__ = ["image_identifier", "image_hash", "created_at", "built_at"]
__repr_attrs__ = [
"image_identifier",
"image_hash",
"created_at",
"built_at",
"config",
]

id: UID
config: WorkerConfig
Expand All @@ -31,9 +38,13 @@ class SyftWorkerImage(SyftObject):

@property
def is_built(self) -> bool:
"""Returns True if the image has been built."""
"""Returns True if the image has been built or is prebuilt."""

return self.built_at is not None
return self.built_at is not None or self.is_prebuilt

@property
def is_prebuilt(self) -> bool:
return isinstance(self.config, PrebuiltWorkerConfig)

@property
def built_image_tag(self) -> Optional[str]:
Expand Down
17 changes: 8 additions & 9 deletions packages/syft/src/syft/service/worker/worker_image_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import contextlib
from typing import List
from typing import Optional
from typing import Tuple
from typing import Union

# third party
Expand Down Expand Up @@ -204,14 +203,14 @@ def get_all(
return SyftError(message=f"{result.err()}")
images: List[SyftWorkerImage] = result.ok()

res: List[Tuple] = []
for im in images:
if im.image_identifier is not None:
res.append((im.image_identifier.full_name_with_tag, im))
else:
# FIXME: syft deployments in kubernetes results in a new image per version
# This results in "default-worker-image" key having multiple values and DictTuple() throws exception
res.append(("default-worker-image", im))
res = {}
# if image is built index by full_name_with_tag
res.update(
{im.image_identifier.full_name_with_tag: im for im in images if im.is_built}
)
# and then index all images by id
# TODO: jupyter repr needs to be updated to show unique values (even if multiple keys point to same value)
res.update({im.id.to_string(): im for im in images if not im.is_built})

return DictTuple(res)

Expand Down
2 changes: 1 addition & 1 deletion packages/syft/tests/syft/custom_worker/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def test_docker_worker_config(dockerfile_path: Path, method: str) -> None:
else:
raise ValueError(f"method must be one of {METHODS}")

assert docker_config.dockerfile == dockerfile_path.read_text()
assert docker_config.dockerfile == dockerfile_path.read_text().strip()
assert docker_config.description == description
new_description = description + " (syft version is 0.8.4-beta.12)"
docker_config.set_description(description_text=new_description)
Expand Down
Loading