-
Notifications
You must be signed in to change notification settings - Fork 6.5k
docs: update resources docs #18991
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
docs: update resources docs #18991
Conversation
| return StopEvent(result="Messages put into memory") | ||
| ``` | ||
|
|
||
| The `Resource` wrapper acts as both a type declaration and an executor. At definition time, it specifies the expected type using `Annotated` - for example, a `Memory` object. At runtime, it invokes the associated factory function, such as `get_memory`, to produce the actual instance. The return type of this function must match the declared type, ensuring consistency between what’s expected and what’s provided during execution. |
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 changed this bit because non technically true, Resource doesn't really declare a type.
| is passed to every step where it's injected. It's important to master this concept because cached and non-cached | ||
| resources can lead to unexpected behaviour, let's see it in detail. | ||
|
|
||
| ## Using Stateful Resources |
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 is the biggest change in this PR. I don't believe we can link statefulness to caching behaviour, addded a paragraph explaining their relationship at the end
|
|
||
| If we disable caching with `Annotated[Memory, Resource(get_memory, cache=False)]`, the function `get_memory` is going | ||
| to be called multiple times but the resource instance will be always the same. Such a resource should be considered | ||
| stateful not regarding its caching behaviour. |
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.
Yup, thanks a lot for bringing this up! When defining stateless VS stateful I didn't think of this, but yeah, this is definitely something that can happen!🙏
|
It's good to go for me, but maybe it would be good if also @logan-markewich took a look at it before merging it :) |
logan-markewich
left a comment
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.
Seems good!
Description
Follow up for resources documentation