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

[BUG] Container Image should be an input into computing the task version for fast registration #4904

Closed
2 tasks done
EngHabu opened this issue Feb 15, 2024 · 2 comments
Closed
2 tasks done
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working flytekit FlyteKit Python related issue

Comments

@EngHabu
Copy link
Contributor

EngHabu commented Feb 15, 2024

Describe the bug

  1. Run pyflyte run --remote on this file:
from flytekit import task, workflow
from flytekit.image_spec import ImageSpec
from flytekitplugins.flyin import vscode

image_spec2 = ImageSpec(
    requirements="requirements.txt",  # python packages to install
    registry="ghcr.io/enghabu/test-flytekit", builder="envd")

@task(container_image=image_spec2)
@vscode(port=8080)
def say_hello(name: str) -> str:
    return f"hello {name}!"

@workflow
def wf2(name: str = "union"):
    say_hello(name=name)
  1. Update the requirements.txt and run again

Error:


RPC Failed, with Status: StatusCode.INVALID_ARGUMENT
        details: task with different structure already exists with id resource_type:TASK project:"flytesnacks" domain:"development" name:"simple_wf.say_hello" version:"a-nzT1l6aEAKm2MYRWfEzg" 
        Debug string UNKNOWN:Error received from peer ipv4:3.130.217.183:443 {grpc_message:"task with different structure already exists with id resource_type:TASK project:\"flytesnacks\" domain:\"development\" name:\"simple_wf.say_hello\" version:\"a-nzT1l6aEAKm2MYRWfEzg\" ", grpc_status:3, created_time:"2024-02-14T20:11:29.658756-08:00"}

Expected behavior

Task version should have changed because the image has changed...

It seems that we compute the digest of the task before creating/uploading the imageSpec which leads to us only discovering the image id pretty much too late into the registration process? Speculating

Additional context to reproduce

cc @jpvotta

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@EngHabu EngHabu added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Feb 15, 2024
Copy link

dosubot bot commented Feb 15, 2024

🤖

Hey @EngHabu! Great to see you around here again. Running into a bit of a snag, I see? Let's dive into it.

Thank you for reporting this issue. It seems like the task versioning in Flyte is not considering changes in the container image as part of the task structure. This could be the reason why you're seeing the error message about a task with a different structure already existing.

In Flyte, task versioning is handled through the discovery_version field in the TaskMetadata message. This field indicates a logical version to apply to the task for the purpose of discovery. When a task is updated, a new version can be assigned to it. This allows Flyte to differentiate between different versions of the same task. You can find more details about this in the tasks.proto file.

However, it seems like the task versioning process does not consider changes in the container image as part of the task structure. This could be why you're seeing the error message about a task with a different structure already existing, even though you've updated the requirements.txt file and the container image.

As for your question about how Flyte handles the registration of a task with an updated requirements.txt file, Flyte incorporates the changes into the Docker image that is used for the task during the process of packaging and registering your tasks and workflows to a Flyte cluster. This is mentioned in the registering_workflows.md document.

I hope this helps clarify the issue. It might be worth considering an enhancement to the task versioning process to include changes in the container image as part of the task structure. This would likely require changes to the Flyte core system, and I would recommend raising this as a feature request.

Sources

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@wild-endeavor
Copy link
Contributor

we hash the file to compute the version, but this file hasn't changed. somehow need to compute the hash of the file after the imagespec hash is available, and then take that (all image spec hashes encountered) into account. not sure if this is easy. @pingsutw would know best.

@eapolinario eapolinario added flytekit FlyteKit Python related issue backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels Feb 15, 2024
@pingsutw pingsutw self-assigned this Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working flytekit FlyteKit Python related issue
Projects
None yet
Development

No branches or pull requests

4 participants