Repository navigation
Add image generation support - #102
dannyjameswilliams wants to merge 8 commits into
Conversation
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
| ImageShape = Literal["square", "landscape", "portrait"] | ||
|
|
||
|
|
||
| class QAImage(BaseModel): |
There was a problem hiding this comment.
we're not using QA shorthand anywhere in code, would be better to explicit full word naming like in other models, too keep consitent?
There was a problem hiding this comment.
Hmm yeah, I don't mind, I just thought QueryAgentImage was a bit of a mouthful. What if we just had Image? It being imported from weaviate.agents.classes makes it clear where it comes from, I suppose
There was a problem hiding this comment.
Yeah, I'd say QueryAgentImage or Image works to align with other model naming
| # type through to `ParsedAskModeResponse[MyModel].final_answer_parsed`. | ||
| M = TypeVar("M", bound=BaseModel) | ||
|
|
||
| _MEDIA_REQUEST_TIMEOUT = 180 # seconds |
There was a problem hiding this comment.
I'm not a fan of pushing the timeout, I'd jsut keep existing default, because technically we wouldn't be able to ensure 100% reliability (e.g. because by default k8s pods would be killed after 30s of termination).
In ideal world you'd switch to async approaches if requests are expected to last that long.
So by keeping original timeout maybe it's better for user to get timeout if it's doing something very heavy (many images) to e.g. split that up to few requests
There was a problem hiding this comment.
The new 2.5 image models are a lot faster, and theoretically a single image + other nodes does fit within a standard QA call in 30s, but if there is a longer final answer then it will likely push above 30s more often than not.
personally I'd like to increase the timeout so a user's first experience with this isn't that it doesn't work and times out. A first time user coming in to test this feature or using it without much API/coding experience may not know to adjust the time out without reading the docs, and it causes a bit of friction I'd like to avoid
by default k8s pods would be killed after 30s of termination
would you mind giving a bit more detail on this? the api call doesn't time out from my testing when it's above 30 seconds, when would a user likely experience this?
2 participants
Related to backend PR #1032
Adds the
GeneratedImageandGeneratedImageOptionsclasses to add to structured outputs.Also adaptively changes the timeout based on if media was requested or not.