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: Calculate the tag based on the name of the base image #2740

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Conversation

pingsutw
Copy link
Member

Tracking issue

NA

Why are the changes needed?

If the base image is an ImageSpec, the flytekit image builder pushes the image to an unexpected place because the tag is not calculated based on the image spec's name.

What changes were proposed in this pull request?

use the name of base ImageSpec to calculate the tag.

How was this patch tested?

Setup process

from flytekit import task, workflow, ImageSpec

image_sklearn = ImageSpec(
    packages=["scikit-learn"],
    apt_packages=["git"],
    registry="ghcr.io/unionai",
    name="flyte-conformance",
)
image_tensorflow = ImageSpec(
    base_image=image_sklearn,
    packages=["tensorflow"],
    registry="ghcr.io/unionai",
    name="flyte-conformance",
)


@task(container_image=image_sklearn)
def t1(a: int) -> int:
    return a + 2


@task(container_image=image_tensorflow)
def t2(a: int) -> int:
    return a + 2


@workflow
def composition_image_wf(a: int = 3):
    t1(a=a)
    t2(a=a)


if __name__ == "__main__":
    print(f"Running my_wf: {composition_image_wf(a=50)}")

Screenshots

Screenshot 2024-09-09 at 10 07 25 PM

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

NA

Docs link

NA

@@ -130,7 +130,7 @@ def tag(self) -> str:
# copy the image spec to avoid modifying the original image spec. otherwise, the hash will be different.
spec = copy.deepcopy(self)
if isinstance(spec.base_image, ImageSpec):
spec = dataclasses.replace(spec, base_image=spec.base_image)
spec = dataclasses.replace(spec, base_image=spec.base_image.image_name())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we used image_name before. I accidentally removed it in this PR

@pingsutw pingsutw merged commit 15d82ef into master Sep 10, 2024
101 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants