Merge pull request #8441 from OpenMined/yash/image-list-fix

Fix DictTuple exception when invoking images.get_all()

notebooks/api/0.8/10-container-images.ipynb

@@ -120,7 +120,7 @@
     "\n",
     "RUN pip install pydicom\n",
     "\n",
-    "\"\"\""
+    "\"\"\".strip()"
    ]
   },
   {
@@ -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",
@@ -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)"
    ]
@@ -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",
@@ -1406,7 +1406,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.11.2"
+   "version": "3.11.7"
   }
  },
  "nbformat": 4,

notebooks/api/0.8/11-container-images-k8s.ipynb

@@ -126,7 +126,7 @@
     "\n",
     "RUN pip install pydicom\n",
     "\n",
-    "\"\"\""
+    "\"\"\".strip()"
    ]
   },
   {
@@ -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",
@@ -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)"
    ]
@@ -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",

packages/syft/src/syft/custom_worker/config.py

@@ -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,

packages/syft/src/syft/service/worker/image_identifier.py

@@ -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})"

packages/syft/src/syft/service/worker/utils.py

@@ -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
@@ -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(

packages/syft/src/syft/service/worker/worker_image.py

@@ -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
@@ -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
@@ -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]:

packages/syft/src/syft/service/worker/worker_image_service.py

@@ -2,7 +2,6 @@
 import contextlib
 from typing import List
 from typing import Optional
-from typing import Tuple
 from typing import Union
 
 # third party
@@ -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)
 

packages/syft/tests/syft/custom_worker/config_test.py

@@ -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)