-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[workflow] Enable setting workflow options on Ray DAGs #24210
Conversation
"name", | ||
"metadata", | ||
"catch_exceptions", | ||
"max_retries", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't max retries be unified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it can be/should be unified. The max_retries in workflow retries at the python program layer and does not catch failures beyond normal exceptions. @iycheng
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics is different. I'd say either leave two APIs here or we give ray's max retries some default value.
LGTM. I feel right now since we have already had to isolate workflow options with ray options, it's reasonable to put |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for @ericl 's confirmation about the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should revisit this later, but it's fine for current stage of workflows.
The CI errors are not related. The MacOS errors are due to |
Why are these changes needed?
This PR enables setting workflow options on Ray DAGs.
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.