-
Notifications
You must be signed in to change notification settings - Fork 669
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
Feature Request: Synchronous Calls #934
Comments
Can you expand on this:
I think we can do a better job than we have so far of powering this workflow better, but I also think we might be able to do that while wrapping the async usage as much as possible. I think we're more likely to run into issues with the But it would definitely be helpful to better understand where the dislike for sync-wrapper-around-async is coming from, if there's anything more to it than just "it's kind of buggy". In particular, I don't want to take on a big maintenance burden of duplicating all code paths in a sync and async way just because there are bugs today that we can fix e.g. by creating a new event loop for every sync call (instead of using the existing one like we currently do). |
I'll also note that the approach described here, which is how psycopg maintains a sync and async version of the codebase, may be feasible for us. Just need to adapt this surprisingly-not-huge script. That said, I think we'll keep the Async version of the code with the non-prefixed names, and add prefixes for Sync code. (Since in most real world usage scenarios, the Async version is what you probably should use.) I think you've raised some good questions in your addendum, I want to address them, but that will take a bit more time and wanted to get a response out to the part of the issue tied to the issue title sooner. |
We agreed on working on this. The idea is that we are going to generate the sync API via an CST script. Whenever I work on my priorities, I'll get back to this. |
I would love it if I had the option use the library entirely with synchronous calls. This way, I have full freedom to enable or disable this, pursue other concurrency models, and would have less issues interfacing with other libraries that are opinionated about open event loops.
Unfortunately,
run.sync
doesn't solve this. I'd actually prefer if synchronous calls were a 1st class citizen and there was a thin wrapper or arg to make things async instead of vice versa. Unfortunately, I understand that this isn't necessarily easy to express in python, meaning it often leads to two full copies of the api for sync and async. Still, I think there's a good reason this is available in e.g. OpenAI's client.Addendum (feel free to ignore)
More broadly (please excuse if I'm overstepping my bounds), but I wonder a bit if this is a downstream issue of a broader problem with what the identity of this library is. When I read:
I think "Oh cool, the Pydantic team is making an agent framework. I'll use that if I need to do something involving agentic workflows, probably overkill otherwise."
But then when I hear:
My thought is, "Oh! Amazing! The Pydantic team is going bring that 'minimal, just works, clean' feel compared to lots of other recent general AI frameworks."
Personally I would love a philosophy that is something like
Overall, I'm sort of hoping for
Pydantic <-> PydanticAI
andFastAPI <-> PydanticAgents
or something like this. But right now they're blending together. I think the tension between these two viewpoints leads to some design consequences:Agent
works great and feels clean. But also there are situations when it seems like you need to create aClient
just to pass it into aModel
just to pass that into anAgent
. Maybe this is outdated docs, but if not, that doesn't feel right. Using a custom endpoint in theOpenAI
client outside of this library requires just passing the different endpoint and api key as args, not instantiating 3 classes. It's not a big deal, but I wonder if more things like this will happen ifAgent
is the first class citizen, instead of viewed as built "on top of"PydanticAI
.Thanks for all the hard work, curious what you think. I wouldn't write this if I wasn't super excited about this library!
The text was updated successfully, but these errors were encountered: