From 505d66cd81d004f075dc5ca3f14a2b7f3d8c1bf5 Mon Sep 17 00:00:00 2001 From: git Date: Thu, 1 Feb 2024 09:08:48 +0000 Subject: [PATCH 1/4] changed build to low level, added api_client --- torchx/runner/api.py | 2 +- torchx/workspace/docker_workspace.py | 32 ++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/torchx/runner/api.py b/torchx/runner/api.py index 67771c9c6..16f73e0f3 100644 --- a/torchx/runner/api.py +++ b/torchx/runner/api.py @@ -375,7 +375,7 @@ def dryrun( ) else: logger.info( - f"Reusing original image `{old_img}` for role[0]={role.name}." + f"Reusing image based on `{old_img}` for role[0]={role.name}." " Either a patch was built or no changes to workspace was detected." ) diff --git a/torchx/workspace/docker_workspace.py b/torchx/workspace/docker_workspace.py index 13d9c7756..3890275d7 100644 --- a/torchx/workspace/docker_workspace.py +++ b/torchx/workspace/docker_workspace.py @@ -20,7 +20,7 @@ from torchx.workspace.api import walk_workspace, WorkspaceMixin if TYPE_CHECKING: - from docker import DockerClient + from docker import DockerClient, APIClient log: logging.Logger = logging.getLogger(__name__) @@ -83,7 +83,15 @@ def _docker_client(self) -> "DockerClient": client = docker.from_env() self.__docker_client = client return client + @property + def _docker_api_client(self) -> "APIClient": + api_client = self.__docker_api_client + if api_client is None: + import docker + api_client = docker.APIClient() + self.__docker_api_client = api_client + return api_client def workspace_opts(self) -> runopts: opts = runopts() opts.add( @@ -119,7 +127,7 @@ def build_workspace_and_update_role( f"failed to pull image {role.image}, falling back to local: {e}" ) log.info("Building workspace docker image (this may take a while)...") - image, _ = self._docker_client.images.build( + build_events = self._docker_api_client.build( fileobj=context, custom_context=True, dockerfile=TORCHX_DOCKERFILE, @@ -132,9 +140,25 @@ def build_workspace_and_update_role( labels={ self.LABEL_VERSION: torchx.__version__, }, + decode=True ) - if len(old_imgs) == 0 or role.image not in old_imgs: - role.image = image.id + while True: + try: + output = build_events.__next__() + if 'stream' in output: + output_str = output['stream'].strip('\r\n').strip('\n') + if output_str != '': + log.info(output_str) + if "aux" in output: + image = output['aux'] + break + except StopIteration: + break + except ValueError: + log.error(f'Error parsing output from docker build: {output}') + + if len(old_imgs) == 0 or image['ID'] not in old_imgs: + role.image = image['ID'] finally: context.close() From 9ff7f09e8332df4b5ecf99169cf0ca2ded4739e8 Mon Sep 17 00:00:00 2001 From: git Date: Sat, 3 Feb 2024 18:52:13 +0000 Subject: [PATCH 2/4] linting --- torchx/workspace/docker_workspace.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/torchx/workspace/docker_workspace.py b/torchx/workspace/docker_workspace.py index 3890275d7..49ade171e 100644 --- a/torchx/workspace/docker_workspace.py +++ b/torchx/workspace/docker_workspace.py @@ -20,7 +20,7 @@ from torchx.workspace.api import walk_workspace, WorkspaceMixin if TYPE_CHECKING: - from docker import DockerClient, APIClient + from docker import APIClient, DockerClient log: logging.Logger = logging.getLogger(__name__) @@ -73,6 +73,7 @@ def __init__( ) -> None: super().__init__(*args, **kwargs) self.__docker_client = docker_client + self.__docker_api_client = None @property def _docker_client(self) -> "DockerClient": @@ -83,6 +84,7 @@ def _docker_client(self) -> "DockerClient": client = docker.from_env() self.__docker_client = client return client + @property def _docker_api_client(self) -> "APIClient": api_client = self.__docker_api_client @@ -92,6 +94,7 @@ def _docker_api_client(self) -> "APIClient": api_client = docker.APIClient() self.__docker_api_client = api_client return api_client + def workspace_opts(self) -> runopts: opts = runopts() opts.add( @@ -140,25 +143,25 @@ def build_workspace_and_update_role( labels={ self.LABEL_VERSION: torchx.__version__, }, - decode=True + decode=True, ) while True: try: output = build_events.__next__() - if 'stream' in output: - output_str = output['stream'].strip('\r\n').strip('\n') - if output_str != '': + if "stream" in output: + output_str = output["stream"].strip("\r\n").strip("\n") + if output_str != "": log.info(output_str) if "aux" in output: - image = output['aux'] + image = output["aux"] break except StopIteration: break except ValueError: - log.error(f'Error parsing output from docker build: {output}') - - if len(old_imgs) == 0 or image['ID'] not in old_imgs: - role.image = image['ID'] + log.error(f"Error parsing output from docker build: {output}") + + if len(old_imgs) == 0 or image["ID"] not in old_imgs: + role.image = image["ID"] finally: context.close() From f965c862da902463b3329e1fdd7b1e24e9140822 Mon Sep 17 00:00:00 2001 From: git Date: Tue, 6 Feb 2024 09:08:53 +0000 Subject: [PATCH 3/4] typing for pyre, uneeded error case, renaming output->build_msg --- torchx/workspace/docker_workspace.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/torchx/workspace/docker_workspace.py b/torchx/workspace/docker_workspace.py index 49ade171e..1b984b994 100644 --- a/torchx/workspace/docker_workspace.py +++ b/torchx/workspace/docker_workspace.py @@ -69,11 +69,12 @@ def __init__( self, *args: object, docker_client: Optional["DockerClient"] = None, + docker_api_client: Optional["APIClient"] = None, **kwargs: object, ) -> None: super().__init__(*args, **kwargs) self.__docker_client = docker_client - self.__docker_api_client = None + self.__docker_api_client = docker_api_client @property def _docker_client(self) -> "DockerClient": @@ -145,20 +146,19 @@ def build_workspace_and_update_role( }, decode=True, ) + build_msg: Dict[str, str] = {} while True: try: - output = build_events.__next__() - if "stream" in output: - output_str = output["stream"].strip("\r\n").strip("\n") + build_msg = build_events.__next__() + if "stream" in build_msg: + output_str = build_msg["stream"].strip("\r\n").strip("\n") if output_str != "": log.info(output_str) - if "aux" in output: - image = output["aux"] + if "aux" in build_msg: + image = build_msg["aux"] break except StopIteration: break - except ValueError: - log.error(f"Error parsing output from docker build: {output}") if len(old_imgs) == 0 or image["ID"] not in old_imgs: role.image = image["ID"] From 62c6eeffb083bdcdd8e49fbb2ae8591a32e495da Mon Sep 17 00:00:00 2001 From: Maatra Date: Wed, 10 Apr 2024 15:45:39 +0100 Subject: [PATCH 4/4] reverted some changes to lose dependency on #819 --- torchx/runner/api.py | 2 +- torchx/workspace/docker_workspace.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/torchx/runner/api.py b/torchx/runner/api.py index 03ae7dbbd..0203a313d 100644 --- a/torchx/runner/api.py +++ b/torchx/runner/api.py @@ -377,7 +377,7 @@ def dryrun( ) else: logger.info( - f"Reusing image based on `{old_img}` for role[0]={role.name}." + f"Reusing original image `{old_img}` for role[0]={role.name}." " Either a patch was built or no changes to workspace was detected." ) diff --git a/torchx/workspace/docker_workspace.py b/torchx/workspace/docker_workspace.py index a847b192f..1d5621364 100644 --- a/torchx/workspace/docker_workspace.py +++ b/torchx/workspace/docker_workspace.py @@ -162,8 +162,8 @@ def build_workspace_and_update_role( except StopIteration: break - if len(old_imgs) == 0 or image["ID"] not in old_imgs: - role.image = image["ID"] + if len(old_imgs) == 0 or role.image not in old_imgs: + role.image = image.id finally: context.close()