-
Notifications
You must be signed in to change notification settings - Fork 660
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
Clarify behavior of interruptible map tasks #5845
Conversation
Removed the retries option from map_tasks.md Signed-off-by: Raghav Mangla <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5845 +/- ##
==========================================
- Coverage 36.71% 36.71% -0.01%
==========================================
Files 1304 1304
Lines 130081 130081
==========================================
- Hits 47764 47758 -6
- Misses 78147 78153 +6
Partials 4170 4170
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@RaghavMangla i don't think this is necessary. instead, could you update the section on task retries to include all the relevant information Dan suggested? |
… retries docs Signed-off-by: RaghavMangla <[email protected]>
Hi, @samhita-alla I have reverted back my changes in map_tasks.md and made suggested changes in task retries docs, can u pls review the changes and suggest if any changes are required |
Signed-off-by: RaghavMangla <[email protected]>
Hi @samhita-alla can u pls review the PR and let me know if any changes are required, the pull request is linked to #4462 |
errors by randomly throwing a `RuntimeError` 5% of the time: | ||
### Understanding Error Types | ||
|
||
Flyte differentiates between two types of errors: |
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.
this section feels disconnected from the retries section. instead of simply copying and pasting, could you please try focusing on making it flow naturally as a single, cohesive section?
Okay, I'll make these changes
…On Thu, 17 Oct, 2024, 08:01 Samhita Alla, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/user_guide/flyte_fundamentals/optimizing_tasks.md
<#5845 (comment)>:
>
-The following version of the `compute_mean` task simulates these kinds of
-errors by randomly throwing a `RuntimeError` 5% of the time:
+### Understanding Error Types
+
+Flyte differentiates between two types of errors:
this section feels disconnected from the retries section. instead of
simply copying and pasting, could you please try focusing on making it flow
naturally as a single, cohesive section?
—
Reply to this email directly, view it on GitHub
<#5845 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXGSZMPC5XG4SEFCCN7IH43Z34ORTAVCNFSM6AAAAABP5QKEL2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNZUGAYTENZXGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi, @samhita-alla I have updated with changes |
Signed-off-by: RaghavMangla <[email protected]>
Hi, @samhita-alla, can u pls provide a review abt the changes required, I have made the changes, and tried to keep the section continuous |
Hi! @neverett @samhita-alla @ppiegaze can u pls review the PR, (this is my fourth PR in hacktoberfest, so a 7 day verification would be required, could u pls share the changes if any today) |
Hi! @samhita-alla, @neverett @ppiegaze, can i get an update about the changes required |
Hi! @samhita-alla @neverett @ppiegaze, can i get an update about the changes required |
Hi, @neverett, I have made all the requested changes as asked, can you pls review the PR, if everything seems to be good, we can close the issue |
system-level or catastrophic errors that may arise from issues that don't have | ||
anything to do with user-defined code, like network issues and data center | ||
outages. | ||
Flyte's robust retry mechanism enhances the reliability of distributed computing environments by effectively managing task failures. This section delves into the configuration and application of retries, ensuring you can maximize task resilience and efficiency. |
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 don't think this block is needed. The old one seems to be more accurate
Also related to #3956 |
Signed-off-by: RaghavMangla <[email protected]>
updated |
Hi! @davidmirror-ops, what other changes are required |
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.
Thanks!
Hi team, can you please add me for contribution in docs |
@RaghavMangla sure, please fill out this form https://tally.so/r/mJJ14r |
Form submitted, thank you for the opportunity! |
Removed the retries option from map_tasks.md
Tracking issue
Closes: #4462
Why are the changes needed?
interruptible should work the same with maptasks as with regular python tasks — the retries field in the task annotation shouldn't be necessary.
What changes were proposed in this pull request?
Removed the retries field in task annotation from documentation, since its not necessary
How was this patch tested?
Setup process
Screenshots
Screenshots of html page rendered for map_tasks.md
Check all the applicable boxes
Related PRs
Docs link
github link: https://github.com/flyteorg/flyte/edit/master/docs/user_guide/advanced_composition/map_tasks.md
official link: https://docs.flyte.org/en/latest/user_guide/advanced_composition/map_tasks.html