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

[Core feature] @dynamic should accept all (?) of @workflow attributes #5465

Open
2 tasks done
EngHabu opened this issue Jun 11, 2024 · 6 comments
Open
2 tasks done

[Core feature] @dynamic should accept all (?) of @workflow attributes #5465

EngHabu opened this issue Jun 11, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@EngHabu
Copy link
Contributor

EngHabu commented Jun 11, 2024

Motivation: Why do you think this is important?

@dynamic represents a dynamic workflow and should accept all configurations we can pass to a static @workflow... I was attempting to add failure_policy and realized it's not possible

Goal: What should the final outcome look like, ideally?

Not yet sure of the UX.
Maybe just @dynamic(...failure_policy...
The downside here is that can potentially conflate the task attributes with workflow ones

Maybe @dynamic(workflow_config=Workflow(failure_policy=....

Describe alternatives you've considered

Map over launchplan. Not supported

Propose: Link/Inline OR Additional context

No response

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@EngHabu EngHabu added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Jun 11, 2024
Copy link

runllm bot commented Jun 11, 2024

Currently, the @dynamic decorator in Flyte does not support all attributes that the @workflow decorator does, such as failure_policy. This limitation has been noted in various discussions and issues within the Flyte community.

According to this discussion, a workaround is to wrap the dynamic workflow within a static workflow that can accept the failure_policy attribute. This way, the failure policy can be applied at the top level and propagate downwards.

For example:

from flytekit import workflow, dynamic, WorkflowFailurePolicy

@workflow(failure_policy=WorkflowFailurePolicy.FAIL_AFTER_EXECUTABLE_NODES_COMPLETE)
def top_level_workflow():
    dynamic_workflow()

@dynamic
def dynamic_workflow():
    # Your dynamic tasks here
    pass

Would you like more details on this workaround or other potential solutions?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Jun 20, 2024
@eapolinario
Copy link
Contributor

we're investing in extending the list of mappable entities and will not prioritize this feature at the moment. I'll keep the issue open in case someone in the community decides to invest in it.

@kumare3
Copy link
Contributor

kumare3 commented Jul 21, 2024

@eapolinario your suggestion and @EngHabu's issue are different isnt it?
@EngHabu I do like @RunLLM's suggestion to just wrap the dynamic in a static workflow? would that be a good workaround?

Copy link

runllm bot commented Jul 21, 2024

Yes, wrapping a dynamic workflow within a static workflow that accepts the failure_policy attribute is a viable workaround. This allows the failure policy to be applied at the top level and propagate downwards to the dynamic workflow.

Example:

from flytekit import workflow, dynamic, WorkflowFailurePolicy

@workflow(failure_policy=WorkflowFailurePolicy.FAIL_AFTER_EXECUTABLE_NODES_COMPLETE)
def top_level_workflow():
    dynamic_workflow()

@dynamic
def dynamic_workflow():
    # Your dynamic tasks here
    pass

Would you like more details on this workaround or other potential solutions?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

@EngHabu
Copy link
Contributor Author

EngHabu commented Jul 21, 2024

@eapolinario your suggestion and @EngHabu's issue are different isnt it? @EngHabu I do like @RunLLM's suggestion to just wrap the dynamic in a static workflow? would that be a good workaround?

No because the subworkflow the dynamic task creates won't propagate these properties. So it's no use.

Copy link

runllm bot commented Jul 21, 2024

The @dynamic decorator in Flyte does not currently support all attributes that the @workflow decorator does, such as failure_policy (source). A common workaround is to wrap the dynamic workflow within a static workflow that can accept the failure_policy attribute, but this does not propagate the properties to sub-workflows created by the dynamic task (source).

Would you like more details on this workaround or other potential solutions?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants