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

EngHabu commented 2 weeks ago

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?

[X] Yes

Have you read the Code of Conduct?

[X] Yes

runllm[bot] commented 2 weeks ago

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 commented 1 week ago

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.

flyteorg / flyte