From 2f0d516adecf006b5af5426a2fc4f771e13b020c Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 27 Aug 2024 20:02:56 +0800 Subject: [PATCH 01/26] feat: support building images with extra copy commands Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 09b874693e..8d47671977 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -75,6 +75,7 @@ $COPY_COMMAND_RUNTIME RUN $RUN_COMMANDS +$EXTRA_COPY_CMDS WORKDIR /root SHELL ["/bin/bash", "-c"] @@ -207,6 +208,13 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): else: run_commands = "" + extra_copy_cmds = "" + if image_spec.copy_src_dest: + for src, dest in image_spec.copy_src_dest: + extra_copy_cmds += f"COPY --chown=flytekit {" ".join(src)} {dest}\n" + else: + extra_copy_cmds = "" + docker_content = DOCKER_FILE_TEMPLATE.substitute( PYTHON_VERSION=python_version, UV_PYTHON_INSTALL_COMMAND=uv_python_install_command, @@ -218,6 +226,7 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): COPY_COMMAND_RUNTIME=copy_command_runtime, ENTRYPOINT=entrypoint, RUN_COMMANDS=run_commands, + EXTRA_COPY_CMDS=extra_copy_cmds, ) dockerfile_path = tmp_dir / "Dockerfile" @@ -247,6 +256,7 @@ class DefaultImageBuilder(ImageSpecBuilder): "pip_extra_index_url", # "registry_config", "commands", + "copy_src_dest", } def build_image(self, image_spec: ImageSpec) -> str: From f675359f1438d7505caf5df2223c87daa9c8447c Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 27 Aug 2024 20:06:33 +0800 Subject: [PATCH 02/26] feat: add `copy` function in ImageSpec Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index 7e2c3acf32..f76405ab4e 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -50,6 +50,7 @@ class ImageSpec: commands: Command to run during the building process tag_format: Custom string format for image tag. The ImageSpec hash passed in as `spec_hash`. For example, to add a "dev" suffix to the image tag, set `tag_format="{spec_hash}-dev"` + copy_src_dest: List of tuples containing source files/directories and their corresponding destination directory for copying. """ name: str = "flytekit" @@ -73,6 +74,7 @@ class ImageSpec: entrypoint: Optional[List[str]] = None commands: Optional[List[str]] = None tag_format: Optional[str] = None + copy_src_dest: Optional[List[Tuple[List[str], str]]] = None def __post_init__(self): self.name = self.name.lower() @@ -232,6 +234,18 @@ def with_apt_packages(self, apt_packages: Union[str, List[str]]) -> "ImageSpec": return new_image_spec + def copy(self, src: List[str], dest: str = ".") -> "ImageSpec": + """ + Builder that returns a new image spec with the source files copied to the destination directory. + """ + new_image_spec = copy.deepcopy(self) + if new_image_spec.copy_src_dest is None: + new_image_spec.copy_src_dest = [] + + new_image_spec.copy_src_dest.append((src, dest)) + + return new_image_spec + def force_push(self) -> "ImageSpec": """ Builder that returns a new image spec with force push enabled. From 4808d32075a59ceacb2bfa18a4adf26fde0a54d4 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 27 Aug 2024 20:17:42 +0800 Subject: [PATCH 03/26] test: build image with extra copy cmds Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_default_builder.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index e61a3cb7c8..40a607cd0a 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -31,7 +31,8 @@ def test_create_docker_context(tmp_path): source_root=os.fspath(source_root), commands=["mkdir my_dir"], entrypoint=["/bin/bash"], - pip_extra_index_url=["https://extra-url.com"] + pip_extra_index_url=["https://extra-url.com"], + copy_src_dest=[(["file1.txt", "/dir1"], "/root")] ) create_docker_context(image_spec, docker_context_path) @@ -49,6 +50,7 @@ def test_create_docker_context(tmp_path): assert "RUN mkdir my_dir" in dockerfile_content assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content assert "mkdir -p $HOME" in dockerfile_content + assert "COPY --chown=flytekit file1.txt /dir1 /root" in dockerfile_content requirements_path = docker_context_path / "requirements_uv.txt" assert requirements_path.exists() @@ -177,6 +179,7 @@ def test_build(tmp_path): requirements=os.fspath(other_requirements_path), source_root=os.fspath(source_root), commands=["mkdir my_dir"], + copy_src_dest=[([f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"], "/my_dir")] ) builder = DefaultImageBuilder() From 53b859c2176640decaab6b3a8097f78d3b9b27ac Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 27 Aug 2024 20:18:11 +0800 Subject: [PATCH 04/26] test: ImageSpec with extra copy cmds Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_image_spec.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/flytekit/unit/core/image_spec/test_image_spec.py b/tests/flytekit/unit/core/image_spec/test_image_spec.py index fa63f08993..a42bcbbd6b 100644 --- a/tests/flytekit/unit/core/image_spec/test_image_spec.py +++ b/tests/flytekit/unit/core/image_spec/test_image_spec.py @@ -26,12 +26,14 @@ def test_image_spec(mock_image_spec_builder): requirements=REQUIREMENT_FILE, registry_config=REGISTRY_CONFIG_FILE, entrypoint=["/bin/bash"], + copy_src_dest=[(["/src/file1.txt"], "/dest")] ) assert image_spec._is_force_push is False image_spec = image_spec.with_commands("echo hello") image_spec = image_spec.with_packages("numpy") image_spec = image_spec.with_apt_packages("wget") + image_spec = image_spec.copy(["/src", "/src/file2.txt"], "/dest") image_spec = image_spec.force_push() assert image_spec.python_version == "3.8" @@ -52,6 +54,7 @@ def test_image_spec(mock_image_spec_builder): assert image_spec.commands == ["echo hello"] assert image_spec._is_force_push is True assert image_spec.entrypoint == ["/bin/bash"] + assert image_spec.copy_src_dest == [(["/src/file1.txt"], "/dest"), (["/src", "/src/file2.txt"], "/dest")] tag = calculate_hash_from_image_spec(image_spec) assert "=" != tag[-1] From 4f9dafe072fd5cfb0b5d17ced3f98156df380f41 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 27 Aug 2024 21:49:17 +0800 Subject: [PATCH 05/26] fix: f-string syntax error in copy command Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 8d47671977..59c2cccc04 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -211,7 +211,8 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): extra_copy_cmds = "" if image_spec.copy_src_dest: for src, dest in image_spec.copy_src_dest: - extra_copy_cmds += f"COPY --chown=flytekit {" ".join(src)} {dest}\n" + src_files_and_dirs = " ".join(src) + extra_copy_cmds += f"COPY --chown=flytekit {src_files_and_dirs} {dest}\n" else: extra_copy_cmds = "" From 76520a00e6388099a28685090aab8a7184604d34 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 4 Sep 2024 20:08:02 +0900 Subject: [PATCH 06/26] fix: rename copy function and related arguments Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 9 ++++----- flytekit/image_spec/image_spec.py | 12 ++++++------ .../unit/core/image_spec/test_default_builder.py | 4 ++-- .../flytekit/unit/core/image_spec/test_image_spec.py | 6 +++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 59c2cccc04..fef8c56d4d 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -209,10 +209,9 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): run_commands = "" extra_copy_cmds = "" - if image_spec.copy_src_dest: - for src, dest in image_spec.copy_src_dest: - src_files_and_dirs = " ".join(src) - extra_copy_cmds += f"COPY --chown=flytekit {src_files_and_dirs} {dest}\n" + if image_spec.copy: + for src in image_spec.copy: + extra_copy_cmds += f'COPY --chown=flytekit {" ".join(src)} /root\n' else: extra_copy_cmds = "" @@ -257,7 +256,7 @@ class DefaultImageBuilder(ImageSpecBuilder): "pip_extra_index_url", # "registry_config", "commands", - "copy_src_dest", + "copy", } def build_image(self, image_spec: ImageSpec) -> str: diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index f76405ab4e..6433c000d5 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -50,7 +50,7 @@ class ImageSpec: commands: Command to run during the building process tag_format: Custom string format for image tag. The ImageSpec hash passed in as `spec_hash`. For example, to add a "dev" suffix to the image tag, set `tag_format="{spec_hash}-dev"` - copy_src_dest: List of tuples containing source files/directories and their corresponding destination directory for copying. + copy: List of files/directories to copy to /root. e.g. ["src/file1.txt", "src/file2.txt"] """ name: str = "flytekit" @@ -74,7 +74,7 @@ class ImageSpec: entrypoint: Optional[List[str]] = None commands: Optional[List[str]] = None tag_format: Optional[str] = None - copy_src_dest: Optional[List[Tuple[List[str], str]]] = None + copy: Optional[List[List[str]]] = None def __post_init__(self): self.name = self.name.lower() @@ -234,15 +234,15 @@ def with_apt_packages(self, apt_packages: Union[str, List[str]]) -> "ImageSpec": return new_image_spec - def copy(self, src: List[str], dest: str = ".") -> "ImageSpec": + def with_copy(self, src: List[str]) -> "ImageSpec": """ Builder that returns a new image spec with the source files copied to the destination directory. """ new_image_spec = copy.deepcopy(self) - if new_image_spec.copy_src_dest is None: - new_image_spec.copy_src_dest = [] + if new_image_spec.copy is None: + new_image_spec.copy = [] - new_image_spec.copy_src_dest.append((src, dest)) + new_image_spec.copy.append(src) return new_image_spec diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index 40a607cd0a..23736de5a2 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -32,7 +32,7 @@ def test_create_docker_context(tmp_path): commands=["mkdir my_dir"], entrypoint=["/bin/bash"], pip_extra_index_url=["https://extra-url.com"], - copy_src_dest=[(["file1.txt", "/dir1"], "/root")] + copy=[["file1.txt", "/dir1"]] ) create_docker_context(image_spec, docker_context_path) @@ -179,7 +179,7 @@ def test_build(tmp_path): requirements=os.fspath(other_requirements_path), source_root=os.fspath(source_root), commands=["mkdir my_dir"], - copy_src_dest=[([f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"], "/my_dir")] + copy=[[f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"]] ) builder = DefaultImageBuilder() diff --git a/tests/flytekit/unit/core/image_spec/test_image_spec.py b/tests/flytekit/unit/core/image_spec/test_image_spec.py index a42bcbbd6b..44596cd4cb 100644 --- a/tests/flytekit/unit/core/image_spec/test_image_spec.py +++ b/tests/flytekit/unit/core/image_spec/test_image_spec.py @@ -26,14 +26,14 @@ def test_image_spec(mock_image_spec_builder): requirements=REQUIREMENT_FILE, registry_config=REGISTRY_CONFIG_FILE, entrypoint=["/bin/bash"], - copy_src_dest=[(["/src/file1.txt"], "/dest")] + copy=[["/src/file1.txt"]] ) assert image_spec._is_force_push is False image_spec = image_spec.with_commands("echo hello") image_spec = image_spec.with_packages("numpy") image_spec = image_spec.with_apt_packages("wget") - image_spec = image_spec.copy(["/src", "/src/file2.txt"], "/dest") + image_spec = image_spec.with_copy(["/src", "/src/file2.txt"]) image_spec = image_spec.force_push() assert image_spec.python_version == "3.8" @@ -54,7 +54,7 @@ def test_image_spec(mock_image_spec_builder): assert image_spec.commands == ["echo hello"] assert image_spec._is_force_push is True assert image_spec.entrypoint == ["/bin/bash"] - assert image_spec.copy_src_dest == [(["/src/file1.txt"], "/dest"), (["/src", "/src/file2.txt"], "/dest")] + assert image_spec.copy == [["/src/file1.txt"], ["/src", "/src/file2.txt"]] tag = calculate_hash_from_image_spec(image_spec) assert "=" != tag[-1] From e390c512604c0d6df96246644b50c5b59d8d00fc Mon Sep 17 00:00:00 2001 From: mao3267 Date: Thu, 5 Sep 2024 07:19:13 +0900 Subject: [PATCH 07/26] fix: update image name hash Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_image_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flytekit/unit/core/image_spec/test_image_spec.py b/tests/flytekit/unit/core/image_spec/test_image_spec.py index c6ac6ff227..b500e8ccc9 100644 --- a/tests/flytekit/unit/core/image_spec/test_image_spec.py +++ b/tests/flytekit/unit/core/image_spec/test_image_spec.py @@ -59,7 +59,7 @@ def test_image_spec(mock_image_spec_builder, monkeypatch): assert image_spec.entrypoint == ["/bin/bash"] assert image_spec.copy == [["/src/file1.txt"], ["/src", "/src/file2.txt"]] - assert image_spec.image_name() == f"localhost:30001/flytekit:lh20ze1E7qsZn5_kBQifRw" + assert image_spec.image_name() == f"localhost:30001/flytekit:Cdsatv_mTV4kwduuC3ul9A" ctx = context_manager.FlyteContext.current_context() with context_manager.FlyteContextManager.with_context( ctx.with_execution_state(ctx.execution_state.with_params(mode=ExecutionState.Mode.TASK_EXECUTION)) From a8bbb1ad4db242454b115bc504ea63e73ff2447e Mon Sep 17 00:00:00 2001 From: mao3267 Date: Thu, 12 Sep 2024 16:20:53 +0800 Subject: [PATCH 08/26] fix: update digest with copy files Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index 561a6b63da..b141e95e9c 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -142,6 +142,14 @@ def tag(self) -> str: digest = compute_digest(self.source_root, ignore.is_ignored) spec = dataclasses.replace(spec, source_root=digest) + if self.copy: + from flytekit.tools.fast_registration import compute_digest + from flytekit.tools.ignore import DockerIgnore, GitIgnore, IgnoreGroup, StandardIgnore + + # ignore = IgnoreGroup("/root", [GitIgnore, DockerIgnore, StandardIgnore]) + digest = compute_digest([path for pathlist in self.copy for path in pathlist], None) # , ignore.is_ignored) + spec = dataclasses.replace(spec, copy=digest) + if spec.requirements: requirements = hashlib.sha1(pathlib.Path(spec.requirements).read_bytes().strip()).hexdigest() spec = dataclasses.replace(spec, requirements=requirements) From e2ec68f4ddc60568ba1b34751e1499c42ca98507 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Thu, 12 Sep 2024 16:21:37 +0800 Subject: [PATCH 09/26] feat: support computing digest from list of files Signed-off-by: mao3267 --- flytekit/tools/fast_registration.py | 51 +++++++++++++++++++---------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/flytekit/tools/fast_registration.py b/flytekit/tools/fast_registration.py index dc3f25bf28..b012b5a60e 100644 --- a/flytekit/tools/fast_registration.py +++ b/flytekit/tools/fast_registration.py @@ -14,7 +14,7 @@ import typing from dataclasses import dataclass from enum import Enum -from typing import Optional +from typing import List, Optional, Union import click from rich import print as rich_print @@ -189,7 +189,7 @@ def fast_package( return archive_fname -def compute_digest(source: os.PathLike, filter: Optional[callable] = None) -> str: +def compute_digest(source: Union[os.PathLike, List[os.PathLike]], filter: Optional[callable] = None) -> str: """ Walks the entirety of the source dir to compute a deterministic md5 hex digest of the dir contents. :param os.PathLike source: @@ -197,22 +197,37 @@ def compute_digest(source: os.PathLike, filter: Optional[callable] = None) -> st :return Text: """ hasher = hashlib.md5() - for root, _, files in os.walk(source, topdown=True): - files.sort() - - for fname in files: - abspath = os.path.join(root, fname) - # Only consider files that exist (e.g. disregard symlinks that point to non-existent files) - if not os.path.exists(abspath): - logger.info(f"Skipping non-existent file {abspath}") - continue - relpath = os.path.relpath(abspath, source) - if filter: - if filter(relpath): - continue - - _filehash_update(abspath, hasher) - _pathhash_update(relpath, hasher) + + def process_file(path: os.PathLike, rel_path: os.PathLike) -> None: + # Only consider files that exist (e.g. disregard symlinks that point to non-existent files) + if not os.path.exists(path): + logger.info(f"Skipping non-existent file {path}") + return + + if filter: + if filter(rel_path): + return + + _filehash_update(path, hasher) + _pathhash_update(rel_path, hasher) + + def process_dir(source: os.PathLike) -> None: + for root, _, files in os.walk(source, topdown=True): + files.sort() + + for fname in files: + abspath = os.path.join(root, fname) + relpath = os.path.relpath(abspath, source) + process_file(abspath, relpath) + + if isinstance(source, list): + for src in source: + if os.path.isdir(src): + process_dir(src) + else: + process_file(src, os.path.basename(src)) + else: + process_dir(source) return hasher.hexdigest() From 02611de0651097061b1b758c921b0108ceb7286f Mon Sep 17 00:00:00 2001 From: mao3267 Date: Thu, 12 Sep 2024 16:22:47 +0800 Subject: [PATCH 10/26] fix: update image tag Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_image_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flytekit/unit/core/image_spec/test_image_spec.py b/tests/flytekit/unit/core/image_spec/test_image_spec.py index b500e8ccc9..9ac321b9c9 100644 --- a/tests/flytekit/unit/core/image_spec/test_image_spec.py +++ b/tests/flytekit/unit/core/image_spec/test_image_spec.py @@ -59,7 +59,7 @@ def test_image_spec(mock_image_spec_builder, monkeypatch): assert image_spec.entrypoint == ["/bin/bash"] assert image_spec.copy == [["/src/file1.txt"], ["/src", "/src/file2.txt"]] - assert image_spec.image_name() == f"localhost:30001/flytekit:Cdsatv_mTV4kwduuC3ul9A" + assert image_spec.image_name() == f"localhost:30001/flytekit:fYU5EUF6y0b2oFG4tu70tA" ctx = context_manager.FlyteContext.current_context() with context_manager.FlyteContextManager.with_context( ctx.with_execution_state(ctx.execution_state.with_params(mode=ExecutionState.Mode.TASK_EXECUTION)) From a61977371d130da048897ae56fa2396898a552a2 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Fri, 13 Sep 2024 12:54:53 +0800 Subject: [PATCH 11/26] fix: remove ignore logic Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index 0d4627708e..d234c1c30d 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -176,10 +176,8 @@ def tag(self) -> str: if self.copy: from flytekit.tools.fast_registration import compute_digest - from flytekit.tools.ignore import DockerIgnore, GitIgnore, IgnoreGroup, StandardIgnore - - # ignore = IgnoreGroup("/root", [GitIgnore, DockerIgnore, StandardIgnore]) - digest = compute_digest([path for pathlist in self.copy for path in pathlist], None) # , ignore.is_ignored) + + digest = compute_digest([path for pathlist in self.copy for path in pathlist], None) spec = dataclasses.replace(spec, copy=digest) if spec.requirements: From e8545f386a3a58a2864403d44f4b757ebca52f77 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Fri, 13 Sep 2024 15:30:28 +0800 Subject: [PATCH 12/26] fix: lint Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index d234c1c30d..1a1a9a2b2f 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -176,7 +176,7 @@ def tag(self) -> str: if self.copy: from flytekit.tools.fast_registration import compute_digest - + digest = compute_digest([path for pathlist in self.copy for path in pathlist], None) spec = dataclasses.replace(spec, copy=digest) From 1c468002fbc8febfc6dcca55e10eebae07c236fe Mon Sep 17 00:00:00 2001 From: mao3267 Date: Sat, 14 Sep 2024 10:04:24 +0800 Subject: [PATCH 13/26] fix: add source_copy_node Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index c69fcc7095..caeb7e9121 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -256,7 +256,7 @@ class DefaultImageBuilder(ImageSpecBuilder): "python_version", "builder", "source_root", - "copy", + "source_copy_mode", "env", "registry", "packages", @@ -272,6 +272,7 @@ class DefaultImageBuilder(ImageSpecBuilder): "pip_extra_index_url", # "registry_config", "commands", + "copy", } def build_image(self, image_spec: ImageSpec) -> str: From eca206354409604e2ede6df8ad4c253bf5f57990 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 17 Sep 2024 22:39:47 +0800 Subject: [PATCH 14/26] fix: copy local files to tmp_dir for docker copy Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index caeb7e9121..6c92412364 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -224,8 +224,19 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): extra_copy_cmds = "" if image_spec.copy: - for src in image_spec.copy: - extra_copy_cmds += f'COPY --chown=flytekit {" ".join(src)} /root\n' + for src_list in image_spec.copy: + dst_path_list = [] + for src in src_list: + src_path = Path(src) + dst_path = tmp_dir / src_path.name + if src_path.is_dir(): + shutil.copytree(src_path, dst_path) + else: + shutil.copy(src_path, dst_path) + + dst_path_list.append(dst_path.name) + + extra_copy_cmds += f"COPY --chown=flytekit {" ".join(dst_path_list)} /root\n" else: extra_copy_cmds = "" From dea9233c07eaf6020bbdf126ca6519f5ad5627d8 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 17 Sep 2024 23:32:57 +0800 Subject: [PATCH 15/26] test: update default builder test and files for copy Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_default_builder.py | 4 ++-- tests/flytekit/unit/core/image_spec/test_files/file.txt | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 tests/flytekit/unit/core/image_spec/test_files/file.txt diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index b25e1d993a..71a03a98ab 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -34,7 +34,7 @@ def test_create_docker_context(tmp_path): entrypoint=["/bin/bash"], pip_extra_index_url=["https://extra-url.com"], source_copy_mode=CopyFileDetection.ALL, - copy=[["file1.txt", "/dir1"]], + copy=[["tests/flytekit/unit/core/image_spec/test_files/file.txt", "tests/flytekit/unit/core/image_spec/test_files"]], ) create_docker_context(image_spec, docker_context_path) @@ -52,7 +52,7 @@ def test_create_docker_context(tmp_path): assert "RUN mkdir my_dir" in dockerfile_content assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content assert "mkdir -p $HOME" in dockerfile_content - assert "COPY --chown=flytekit file1.txt /dir1 /root" in dockerfile_content + assert "COPY --chown=flytekit file.txt test_files /root" in dockerfile_content requirements_path = docker_context_path / "requirements_uv.txt" assert requirements_path.exists() diff --git a/tests/flytekit/unit/core/image_spec/test_files/file.txt b/tests/flytekit/unit/core/image_spec/test_files/file.txt new file mode 100644 index 0000000000..321580deb3 --- /dev/null +++ b/tests/flytekit/unit/core/image_spec/test_files/file.txt @@ -0,0 +1 @@ +Flytekit \ No newline at end of file From 954b27b875f48a2f2d6baecc94c9d8532e50e831 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 17 Sep 2024 23:49:57 +0800 Subject: [PATCH 16/26] fix: f-string related error Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 6c92412364..45c3127d5c 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -236,7 +236,7 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): dst_path_list.append(dst_path.name) - extra_copy_cmds += f"COPY --chown=flytekit {" ".join(dst_path_list)} /root\n" + extra_copy_cmds += f'COPY --chown=flytekit {" ".join(dst_path_list)} /root\n' else: extra_copy_cmds = "" From 4d5215ce236019237b2b84a0f6506bf9c0ded705 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Tue, 17 Sep 2024 23:50:25 +0800 Subject: [PATCH 17/26] fix: lint Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_files/file.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flytekit/unit/core/image_spec/test_files/file.txt b/tests/flytekit/unit/core/image_spec/test_files/file.txt index 321580deb3..e8252f0c46 100644 --- a/tests/flytekit/unit/core/image_spec/test_files/file.txt +++ b/tests/flytekit/unit/core/image_spec/test_files/file.txt @@ -1 +1 @@ -Flytekit \ No newline at end of file +Flytekit From 8f0e3afb5ae69a05ea625a4b73753465715e0d69 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 18 Sep 2024 10:11:33 +0800 Subject: [PATCH 18/26] feat: support list of strings and strings in `with_copy` Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index 1a1a9a2b2f..be17819780 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -85,7 +85,7 @@ class ImageSpec: commands: Optional[List[str]] = None tag_format: Optional[str] = None source_copy_mode: Optional[CopyFileDetection] = None - copy: Optional[List[List[str]]] = None + copy: Optional[List[str]] = None def __post_init__(self): self.name = self.name.lower() @@ -177,7 +177,7 @@ def tag(self) -> str: if self.copy: from flytekit.tools.fast_registration import compute_digest - digest = compute_digest([path for pathlist in self.copy for path in pathlist], None) + digest = compute_digest(self.copy, None) spec = dataclasses.replace(spec, copy=digest) if spec.requirements: @@ -307,7 +307,7 @@ def with_apt_packages(self, apt_packages: Union[str, List[str]]) -> "ImageSpec": new_image_spec = self._update_attribute("apt_packages", apt_packages) return new_image_spec - def with_copy(self, src: List[str]) -> "ImageSpec": + def with_copy(self, src: Union[str, List[str]]) -> "ImageSpec": """ Builder that returns a new image spec with the source files copied to the destination directory. """ @@ -315,7 +315,10 @@ def with_copy(self, src: List[str]) -> "ImageSpec": if new_image_spec.copy is None: new_image_spec.copy = [] - new_image_spec.copy.append(src) + if isinstance(src, list): + new_image_spec.copy.extend(src) + else: + new_image_spec.copy.append(src) return new_image_spec From 3b4070dd34e8c241666850a69b6976e4d8c9ffde Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 18 Sep 2024 10:12:32 +0800 Subject: [PATCH 19/26] fix: change ImageSpec.copy to list[str] Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 25 +++++++++---------- .../core/image_spec/test_default_builder.py | 4 +-- .../unit/core/image_spec/test_image_spec.py | 4 +-- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 45c3127d5c..948a8117ce 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -224,19 +224,18 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): extra_copy_cmds = "" if image_spec.copy: - for src_list in image_spec.copy: - dst_path_list = [] - for src in src_list: - src_path = Path(src) - dst_path = tmp_dir / src_path.name - if src_path.is_dir(): - shutil.copytree(src_path, dst_path) - else: - shutil.copy(src_path, dst_path) - - dst_path_list.append(dst_path.name) - - extra_copy_cmds += f'COPY --chown=flytekit {" ".join(dst_path_list)} /root\n' + dst_file_list = [] + for src in image_spec.copy: + src_path = Path(src) + dst_path = tmp_dir / src_path.name + if src_path.is_dir(): + shutil.copytree(src_path, dst_path) + else: + shutil.copy(src_path, dst_path) + + dst_file_list.append(dst_path.name) + + extra_copy_cmds += f'COPY --chown=flytekit {" ".join(dst_file_list)} /root\n' else: extra_copy_cmds = "" diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index 71a03a98ab..79789c54e4 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -34,7 +34,7 @@ def test_create_docker_context(tmp_path): entrypoint=["/bin/bash"], pip_extra_index_url=["https://extra-url.com"], source_copy_mode=CopyFileDetection.ALL, - copy=[["tests/flytekit/unit/core/image_spec/test_files/file.txt", "tests/flytekit/unit/core/image_spec/test_files"]], + copy=["tests/flytekit/unit/core/image_spec/test_files/file.txt", "tests/flytekit/unit/core/image_spec/test_files"], ) create_docker_context(image_spec, docker_context_path) @@ -181,7 +181,7 @@ def test_build(tmp_path): requirements=os.fspath(other_requirements_path), source_root=os.fspath(source_root), commands=["mkdir my_dir"], - copy=[[f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"]] + copy=[f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"] ) builder = DefaultImageBuilder() diff --git a/tests/flytekit/unit/core/image_spec/test_image_spec.py b/tests/flytekit/unit/core/image_spec/test_image_spec.py index 89b9c310f6..e949b5c7f7 100644 --- a/tests/flytekit/unit/core/image_spec/test_image_spec.py +++ b/tests/flytekit/unit/core/image_spec/test_image_spec.py @@ -32,7 +32,7 @@ def test_image_spec(mock_image_spec_builder, monkeypatch): requirements=REQUIREMENT_FILE, registry_config=REGISTRY_CONFIG_FILE, entrypoint=["/bin/bash"], - copy=[["/src/file1.txt"]] + copy=["/src/file1.txt"] ) assert image_spec._is_force_push is False @@ -60,7 +60,7 @@ def test_image_spec(mock_image_spec_builder, monkeypatch): assert image_spec.commands == ["echo hello"] assert image_spec._is_force_push is True assert image_spec.entrypoint == ["/bin/bash"] - assert image_spec.copy == [["/src/file1.txt"], ["/src", "/src/file2.txt"]] + assert image_spec.copy == ["/src/file1.txt", "/src", "/src/file2.txt"] assert image_spec.image_name() == f"localhost:30001/flytekit:fYU5EUF6y0b2oFG4tu70tA" ctx = context_manager.FlyteContext.current_context() From b7e37dd77502bf352d1b65fe0858430b3ad67006 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Sat, 21 Sep 2024 20:00:37 +0800 Subject: [PATCH 20/26] fix: remain source tree structure Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 24 ++++++++++++------- .../core/image_spec/test_default_builder.py | 3 ++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 948a8117ce..32778ede6e 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -183,11 +183,11 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): ) for file_to_copy in ls: - rel_path = os.path.relpath(file_to_copy, start=str(image_spec.source_root)) - Path(source_path / rel_path).parent.mkdir(parents=True, exist_ok=True) + src_path = os.path.relpath(file_to_copy, start=str(image_spec.source_root)) + Path(source_path / src_path).parent.mkdir(parents=True, exist_ok=True) shutil.copy( file_to_copy, - source_path / rel_path, + source_path / src_path, ) copy_command_runtime = "COPY --chown=flytekit ./src /root" @@ -224,18 +224,24 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): extra_copy_cmds = "" if image_spec.copy: - dst_file_list = [] + copy_commands = [] for src in image_spec.copy: src_path = Path(src) - dst_path = tmp_dir / src_path.name + + if src_path.is_absolute() or ".." in src_path.parts: + raise ValueError("Absolute paths or paths with '..' are not allowed in COPY command.") + + dst_path = tmp_dir / src_path + dst_path.parent.mkdir(parents=True, exist_ok=True) + if src_path.is_dir(): - shutil.copytree(src_path, dst_path) + shutil.copytree(src_path, dst_path, dirs_exist_ok=True) + copy_commands.append(f"COPY --chown=flytekit {src_path} /root/{src_path}/") else: shutil.copy(src_path, dst_path) + copy_commands.append(f"COPY --chown=flytekit {src_path} /root/{src_path.parent}/") - dst_file_list.append(dst_path.name) - - extra_copy_cmds += f'COPY --chown=flytekit {" ".join(dst_file_list)} /root\n' + extra_copy_cmds = "\n".join(copy_commands) else: extra_copy_cmds = "" diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index 79789c54e4..8ab5b9a91a 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -52,7 +52,8 @@ def test_create_docker_context(tmp_path): assert "RUN mkdir my_dir" in dockerfile_content assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content assert "mkdir -p $HOME" in dockerfile_content - assert "COPY --chown=flytekit file.txt test_files /root" in dockerfile_content + assert "COPY --chown=flytekit tests/flytekit/unit/core/image_spec/test_files/file.txt /root/tests/flytekit/unit/core/image_spec/test_files/" in dockerfile_content + assert "COPY --chown=flytekit tests/flytekit/unit/core/image_spec/test_files /root/tests/flytekit/unit/core/image_spec/" in dockerfile_content requirements_path = docker_context_path / "requirements_uv.txt" assert requirements_path.exists() From 53ddf882f5b8bdd45f4ebc5704c0acb7dad9837d Mon Sep 17 00:00:00 2001 From: mao3267 Date: Sat, 21 Sep 2024 20:31:56 +0800 Subject: [PATCH 21/26] fix: path separator diff on windows Signed-off-by: mao3267 --- flytekit/image_spec/default_builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 32778ede6e..742a6bc46c 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -236,10 +236,10 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): if src_path.is_dir(): shutil.copytree(src_path, dst_path, dirs_exist_ok=True) - copy_commands.append(f"COPY --chown=flytekit {src_path} /root/{src_path}/") + copy_commands.append(f"COPY --chown=flytekit {src_path.as_posix()} /root/{src_path.as_posix()}/") else: shutil.copy(src_path, dst_path) - copy_commands.append(f"COPY --chown=flytekit {src_path} /root/{src_path.parent}/") + copy_commands.append(f"COPY --chown=flytekit {src_path.as_posix()} /root/{src_path.parent.as_posix()}/") extra_copy_cmds = "\n".join(copy_commands) else: From e9b1c96cbc3acb63995a8773bf900436e171c271 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 25 Sep 2024 14:39:05 +0800 Subject: [PATCH 22/26] fix: use update_attribute instead Signed-off-by: mao3267 --- flytekit/image_spec/image_spec.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/flytekit/image_spec/image_spec.py b/flytekit/image_spec/image_spec.py index 6dd13f99b7..5af5a5c2de 100644 --- a/flytekit/image_spec/image_spec.py +++ b/flytekit/image_spec/image_spec.py @@ -318,16 +318,7 @@ def with_copy(self, src: Union[str, List[str]]) -> "ImageSpec": """ Builder that returns a new image spec with the source files copied to the destination directory. """ - new_image_spec = copy.deepcopy(self) - if new_image_spec.copy is None: - new_image_spec.copy = [] - - if isinstance(src, list): - new_image_spec.copy.extend(src) - else: - new_image_spec.copy.append(src) - - return new_image_spec + return self._update_attribute("copy", src) def force_push(self) -> "ImageSpec": """ From ff7699f063d059a4e1def0b9cad1dc513d456a68 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 25 Sep 2024 14:39:32 +0800 Subject: [PATCH 23/26] rm: remove test files Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_files/file.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 tests/flytekit/unit/core/image_spec/test_files/file.txt diff --git a/tests/flytekit/unit/core/image_spec/test_files/file.txt b/tests/flytekit/unit/core/image_spec/test_files/file.txt deleted file mode 100644 index e8252f0c46..0000000000 --- a/tests/flytekit/unit/core/image_spec/test_files/file.txt +++ /dev/null @@ -1 +0,0 @@ -Flytekit From b1ee46a067cfdaf016428e1045aa585f6ad44638 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 25 Sep 2024 14:40:07 +0800 Subject: [PATCH 24/26] fix: test with tmp dirs/files Signed-off-by: mao3267 --- .../core/image_spec/test_default_builder.py | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index 8ab5b9a91a..0e5755bffe 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -7,7 +7,8 @@ from flytekit.image_spec import ImageSpec from flytekit.image_spec.default_builder import DefaultImageBuilder, create_docker_context from flytekit.constants import CopyFileDetection - +from pathlib import Path +import tempfile def test_create_docker_context(tmp_path): docker_context_path = tmp_path / "builder_root" @@ -21,23 +22,27 @@ def test_create_docker_context(tmp_path): other_requirements_path = tmp_path / "requirements.txt" other_requirements_path.write_text("threadpoolctl\n") - image_spec = ImageSpec( - name="FLYTEKIT", - python_version="3.12", - env={"MY_ENV": "MY_VALUE"}, - apt_packages=["curl"], - conda_packages=["scipy==1.13.0", "numpy"], - packages=["pandas==2.2.1"], - requirements=os.fspath(other_requirements_path), - source_root=os.fspath(source_root), - commands=["mkdir my_dir"], - entrypoint=["/bin/bash"], - pip_extra_index_url=["https://extra-url.com"], - source_copy_mode=CopyFileDetection.ALL, - copy=["tests/flytekit/unit/core/image_spec/test_files/file.txt", "tests/flytekit/unit/core/image_spec/test_files"], - ) + with tempfile.TemporaryDirectory(dir=Path.cwd().as_posix()) as tmp_dir: + tmp_file = Path(tmp_dir) / "copy_file.txt" + tmp_file.write_text("copy_file_content") + + image_spec = ImageSpec( + name="FLYTEKIT", + python_version="3.12", + env={"MY_ENV": "MY_VALUE"}, + apt_packages=["curl"], + conda_packages=["scipy==1.13.0", "numpy"], + packages=["pandas==2.2.1"], + requirements=os.fspath(other_requirements_path), + source_root=os.fspath(source_root), + commands=["mkdir my_dir"], + entrypoint=["/bin/bash"], + pip_extra_index_url=["https://extra-url.com"], + source_copy_mode=CopyFileDetection.ALL, + copy=[tmp_file.relative_to(Path.cwd().as_posix())], + ) - create_docker_context(image_spec, docker_context_path) + create_docker_context(image_spec, docker_context_path) dockerfile_path = docker_context_path / "Dockerfile" assert dockerfile_path.exists() @@ -52,8 +57,7 @@ def test_create_docker_context(tmp_path): assert "RUN mkdir my_dir" in dockerfile_content assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content assert "mkdir -p $HOME" in dockerfile_content - assert "COPY --chown=flytekit tests/flytekit/unit/core/image_spec/test_files/file.txt /root/tests/flytekit/unit/core/image_spec/test_files/" in dockerfile_content - assert "COPY --chown=flytekit tests/flytekit/unit/core/image_spec/test_files /root/tests/flytekit/unit/core/image_spec/" in dockerfile_content + assert f"COPY --chown=flytekit {tmp_file.relative_to(Path.cwd().as_posix())} /root/" in dockerfile_content requirements_path = docker_context_path / "requirements_uv.txt" assert requirements_path.exists() From 4e347b9484195d3aad5ab40898e6eb20937b7d57 Mon Sep 17 00:00:00 2001 From: mao3267 Date: Wed, 25 Sep 2024 15:18:48 +0800 Subject: [PATCH 25/26] fix: path separator error Signed-off-by: mao3267 --- tests/flytekit/unit/core/image_spec/test_default_builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/flytekit/unit/core/image_spec/test_default_builder.py b/tests/flytekit/unit/core/image_spec/test_default_builder.py index 0e5755bffe..dacce33f01 100644 --- a/tests/flytekit/unit/core/image_spec/test_default_builder.py +++ b/tests/flytekit/unit/core/image_spec/test_default_builder.py @@ -39,7 +39,7 @@ def test_create_docker_context(tmp_path): entrypoint=["/bin/bash"], pip_extra_index_url=["https://extra-url.com"], source_copy_mode=CopyFileDetection.ALL, - copy=[tmp_file.relative_to(Path.cwd().as_posix())], + copy=[tmp_file.relative_to(Path.cwd()).as_posix()], ) create_docker_context(image_spec, docker_context_path) @@ -57,7 +57,7 @@ def test_create_docker_context(tmp_path): assert "RUN mkdir my_dir" in dockerfile_content assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content assert "mkdir -p $HOME" in dockerfile_content - assert f"COPY --chown=flytekit {tmp_file.relative_to(Path.cwd().as_posix())} /root/" in dockerfile_content + assert f"COPY --chown=flytekit {tmp_file.relative_to(Path.cwd()).as_posix()} /root/" in dockerfile_content requirements_path = docker_context_path / "requirements_uv.txt" assert requirements_path.exists() From a15940ddb12cafd1302380782f5b2b774f1013d6 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 25 Sep 2024 16:06:37 -0700 Subject: [PATCH 26/26] nit Signed-off-by: Kevin Su --- flytekit/image_spec/default_builder.py | 7 +++---- flytekit/tools/fast_registration.py | 12 ++++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/flytekit/image_spec/default_builder.py b/flytekit/image_spec/default_builder.py index 742a6bc46c..9bf8dc3852 100644 --- a/flytekit/image_spec/default_builder.py +++ b/flytekit/image_spec/default_builder.py @@ -183,11 +183,11 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): ) for file_to_copy in ls: - src_path = os.path.relpath(file_to_copy, start=str(image_spec.source_root)) - Path(source_path / src_path).parent.mkdir(parents=True, exist_ok=True) + rel_path = os.path.relpath(file_to_copy, start=str(image_spec.source_root)) + Path(source_path / rel_path).parent.mkdir(parents=True, exist_ok=True) shutil.copy( file_to_copy, - source_path / src_path, + source_path / rel_path, ) copy_command_runtime = "COPY --chown=flytekit ./src /root" @@ -222,7 +222,6 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path): else: run_commands = "" - extra_copy_cmds = "" if image_spec.copy: copy_commands = [] for src in image_spec.copy: diff --git a/flytekit/tools/fast_registration.py b/flytekit/tools/fast_registration.py index 3a560d1c8f..989be2f5bd 100644 --- a/flytekit/tools/fast_registration.py +++ b/flytekit/tools/fast_registration.py @@ -179,7 +179,7 @@ def compute_digest(source: Union[os.PathLike, List[os.PathLike]], filter: Option """ hasher = hashlib.md5() - def process_file(path: os.PathLike, rel_path: os.PathLike) -> None: + def compute_digest_for_file(path: os.PathLike, rel_path: os.PathLike) -> None: # Only consider files that exist (e.g. disregard symlinks that point to non-existent files) if not os.path.exists(path): logger.info(f"Skipping non-existent file {path}") @@ -192,23 +192,23 @@ def process_file(path: os.PathLike, rel_path: os.PathLike) -> None: _filehash_update(path, hasher) _pathhash_update(rel_path, hasher) - def process_dir(source: os.PathLike) -> None: + def compute_digest_for_dir(source: os.PathLike) -> None: for root, _, files in os.walk(source, topdown=True): files.sort() for fname in files: abspath = os.path.join(root, fname) relpath = os.path.relpath(abspath, source) - process_file(abspath, relpath) + compute_digest_for_file(abspath, relpath) if isinstance(source, list): for src in source: if os.path.isdir(src): - process_dir(src) + compute_digest_for_dir(src) else: - process_file(src, os.path.basename(src)) + compute_digest_for_file(src, os.path.basename(src)) else: - process_dir(source) + compute_digest_for_dir(source) return hasher.hexdigest()