docs: added notes to ApplicationError and FailureError exception docstrings.
#718
Conversation
|
|
|
|
GSmithApps
force-pushed
the
docs-added-notes-to-exception-docstrings
branch
from
2c6c45b to
41d13a7
Compare
There was a problem hiding this comment.
@dandavison - any thoughts/suggestions on API docs for explaining exceptions vs README or docs.temporal.io?
temporalio/exceptions.py
Outdated
| Any exception that does not extend this exception | ||
| is considered a Workflow Task Failure. These types of failures will cause the Workflow Task to be retried. |
There was a problem hiding this comment.
It's actually more complicated than this since it is now configurable. We dedicate a section of our README to this at https://github.com/temporalio/sdk-python?tab=readme-ov-file#exceptions. Ideally we don't have to duplicate so much in here and there considering they may not say the same things, as is the case already here where it is not explained that this is customizable.
|
I like the direction of this PR, but would like to work on the wording a bit before merging.
IMO the information added in this PR is basic and important enough that it should be in the code (API docs) as this PR does. Ideally it should also be accurately duplicated in the docs site. I think the fact that this is customizable should definitely be noted, but should not distract us from communicating the basic Temporal exception philosophy that failing the workflow execution requires special exceptions to be raised and that, by default, unhandled exceptions fail the task. |
|
This is awesome - thank you guys so much for looking at this. Dan, I committed your suggestions 👍 One other thing I think we should include is a note about manually failing an Activity. For example, if a developer/user wants to fail an Activity Task Execution (or I guess the execution as a whole), shouldn't they use an Application Error? In our notes here, we only mention things about failing Workflow Executions. Lastly, I think we'll have similar changes in the other SDKs, for example Go 👍 |
Co-authored-by: Dan Davison <[email protected]>
Co-authored-by: Dan Davison <[email protected]>
|
Please forgive me if I'm missing something obvious, but do either of you know why the pipeline is failing? I'm pretty sure I'm making the linter happy, and this is failing after 13 minutes and 10,000 lines of logs. Also, I tried rerunning it, and I'm getting the same error. |
This isn't your fault! The test suite has some flakes which we need to address. Unfortunately for now we just have to re-run until they pass. (Not sure yet what happened with the features tests) |
|
Hi @dandavison , thanks for the note on the pipeline, and thanks for the notes in general! I've tried five times, and I can't seem to get this last piece to pass. Maybe we can try tomorrow or something -- I'm not sure if you have any tricks up your sleeve. In any case, I'm happy with this however it turns out, so please feel free to make any modifications or merge however you please. I'll stay out of your hair and let you and the experts take it from here if that works 👍🏼 Certainly please do reach out though if there's any remaining legwork -- I'd be happy to take care of it. |
What was changed
Added notes to exception docstrings.
Why?
Increase clarity and dicsoverability.