Release 0 5 0 - #91
Conversation
Investigate generate text payload
feat: make text gen. async
Add Sandbox methods
feat: add indexing v2
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d26432298c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not wait: | ||
| data = response.json().get("data") | ||
| if data is not None: | ||
| return data | ||
| return response.json() |
There was a problem hiding this comment.
Keep API failures visible when wait=False
When wait=False is used by the new async callers such as Video.index() and Video.understand(), a 200 JSON error response with success: false bypasses _parse_response, so the SDK returns the raw error body or None instead of raising InvalidRequestError. Invalid async requests then look like missing IDs and callers cannot distinguish server rejection from no result; please still validate the success flag here and only skip the polling step.
Useful? React with 👍 / 👎.
| path=f"{ApiPath.sandbox}/{self.id}/stop", | ||
| data={"grace": grace}, | ||
| ) | ||
| self._update(data.get("data", {})) |
There was a problem hiding this comment.
Update sandbox from the parsed stop response
HttpClient.post() already returns the response's data object, and the other sandbox methods pass that parsed object directly into _update; looking for another nested data key here leaves the local Sandbox status/name timestamps unchanged after stop() when the API returns the normal sandbox payload. Users who call sandbox.stop() will continue to see the stale pre-stop status until they manually refresh, so this should update from data or {} instead.
Useful? React with 👍 / 👎.
| return Image(self._connection, **image_data) | ||
| if not image_data: | ||
| return None | ||
| if image_data.get("job_id"): |
There was a problem hiding this comment.
Accept documented async id before building assets
The async response schema documented in this commit exposes the job identifier as data.id, and GenerationJob.from_data() already accepts id, but this branch only recognizes job_id. When FLUX/OmniVoice returns the documented async payload, this falls through and tries to construct an Image/Audio without asset fields like collection_id, so callers get a constructor error instead of a GenerationJob; check for job_id or id here and in generate_voice.
Useful? React with 👍 / 👎.
| """Poll an output URL until the job completes or times out.""" | ||
| start = time.monotonic() | ||
| while True: | ||
| response_json = self.session.get(url).json() |
There was a problem hiding this comment.
Retry transient output polling failures
With the backoff wrapper removed, any transient failure while polling the output URL, such as a temporary non-JSON/5xx response or connection hiccup during a long-running synchronous operation, now escapes from this unguarded session.get(...).json() immediately instead of being retried until the poll timeout. That can fail an otherwise successful job client-side on a single bad poll response; keep retrying retryable poll fetch/parsing errors inside this loop.
Useful? React with 👍 / 👎.
| :return: Details of the async response | ||
| :rtype: dict | ||
| """ | ||
| return self.get(path=f"{ApiPath.async_response}/{id}", wait=False) |
There was a problem hiding this comment.
Preserve async response status while polling
get_async_response() is the method callers would use to poll /async-response/{id}, but passing wait=False through the generic client returns only the data object whenever it exists and drops the top-level status/success fields. For processing or failed async responses, callers then cannot tell whether the operation is still running, done, or failed, so this method should preserve the response wrapper instead of unwrapping data.
Useful? React with 👍 / 👎.
|
|
||
| @property | ||
| def is_ready(self): | ||
| return self.status in (SandboxStatus.provisioning, SandboxStatus.active) |
There was a problem hiding this comment.
Report ready only after the sandbox is active
A newly created sandbox remains in provisioning until wait_for_ready() observes active, but this property reports True for provisioning as well. Code that checks sandbox.is_ready before routing self-inference jobs can send work to a compute pool that is not active yet, so the readiness predicate should match the active state (or be renamed to avoid implying job readiness).
Useful? React with 👍 / 👎.
Remove the complete SDK change set bundled by d855fbc on the release-0-5-0 lineage, including sandbox APIs and the coupled async polling and large-prompt upload changes. Preserve the independently merged Indexing V2 and face-store API surfaces where they do not depend on the reverted HTTP behavior.
53ba8c0 to
78d20ed
Compare
Remove FaceStore, Identity, and Face APIs and their collection routes. Move the generic IndexResult model to videodb.index so video indexing remains available without the face-store module.
No description provided.