-
Notifications
You must be signed in to change notification settings - Fork 115
POC For a simple plugin #1035
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
base: main
Are you sure you want to change the base?
POC For a simple plugin #1035
Conversation
workflow_runner: WorkflowRunner | ||
|
||
|
||
class Plugin( |
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.
Can you show the OpenAI and Pydantic plugins written with this form of plugin compared to how they are written now? Can we then compare what we saved users by having two ways to do the same thing? If we recognize they can't use some of this stuff because, say, we ignored that other options may be needed (e.g. additive sandbox customization) or ignored that they may need something for replayer not just worker, did we really help? Sure we can go back and "oh yeah, oops forgot that" all we want forever, but that doesn't not make a stable, easy to understand interface.
So now, arguably, we have done the opposite. We took what was a way, like interceptors, to configure things, and now made two ways, one of which doesn't get the job done. That's going backwards.
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 added the OpenAI agents version of a simple plugin, so that should speak to your first point. My expectation would not be "Oops forgot that" but rather, that isn't supported in this form - go use the low level one, unless we see a very common need.
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.
Additive sandbox customization is one I considered adding, but I wanted to just get something out so we can decide if we want to do it at all before polishing.
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.
go use the low level one
Went from "pit of success" to "abandon this pit and go use a different one" heh. I do think, if we have to support a simpler declarative form here, we could do so inside the current construct, but hopefully the current plugin construct can stand on its own and continue to be the sole plugin approach. It has already been used successfully by users.
What was changed
Why?
Checklist
Closes
How was this tested: