Repository navigation
Conversation
PolicyVersion.base_version_id is typed None | UUID, but the policy
version wrapper replaced a null value with the all-zero UUID before
parsing. Callers could not tell "no base version" apart from a real
one. The zero-UUID substitution dates from when the generated field was
non-nullable; now a null stays None (a missing key is still tolerated).
Tested: new test fails before (got UUID('00000000-...')) and passes
after; uv run pytest, ruff check, ruff format --check all pass.
translate_response took "detail", "error" or "message" from a JSON
error body as-is, so a body like {"detail": ["a", "b"]} or
{"error": {...}} produced an exception whose .message was a list or
dict, despite the str annotation. A list is now joined with "; " (as
list bodies already are) and any other non-string value is str()'d.
Tested: new parametrized test fails before (message was ['a', 'b'] /
{'code': 'bad'}) and passes after; uv run pytest, ruff check, ruff
format --check all pass.
FileUpload declared its Pydantic settings with the class-based `class Config`, which Pydantic 2 deprecates and which emits a PydanticDeprecatedSince20 warning every time roe.models.file is imported. Switch to model_config = ConfigDict(...) with the same setting (arbitrary_types_allowed=True). Tested: new test importing roe.models.file under -W error::DeprecationWarning fails before and passes after; the pytest warning summary is now empty; uv run pytest, ruff check, ruff format --check all pass.
file_obj is typed typing.BinaryIO, and with arbitrary_types_allowed pydantic validates it with isinstance(). Real file objects (open(..., "rb"), io.BytesIO) are not instances of typing.BinaryIO, so every FileUpload(file_obj=...) raised ValidationError and the file-object path was unusable. Keep the annotation but skip validation for this field. Tested: new test in tests/unit/test_file_upload.py fails before (ValidationError: Input should be an instance of BinaryIO), passes after; full suite, ruff check and ruff format --check pass.
 |
429 fell through to the base RoeAPIException, so callers couldn't catch rate limiting by type. RateLimitError subclasses RoeAPIException, so existing handlers still match. Part of roe-ai#88.
The transport retried rate-limited requests after 1-4s regardless of the server's Retry-After. Wait for an integer Retry-After (capped at 60s) when it exceeds the backoff; HTTP-date values keep the backoff. Part of roe-ai#88.
jadenfix
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One commit per fix:
base_version_idcame back as the all-zero UUID, so callers couldn't tell "no base version" from a real one. The generatedPolicyVersionalready types itNone | UUID; it's nowNone.RoeAPIException.messagecould be a list or dict. A list indetail/error/messageis now joined with "; " (as list bodies already are), and other non-string values go throughstr().FileUpload(file_obj=...)rejected every real file object. The field is typedtyping.BinaryIOand pydantic checks it withisinstance, whichopen(..., "rb")andio.BytesIOboth fail. Validation is skipped for that field; the annotation is unchanged.FileUploadused pydantic's deprecated class-basedConfig, which warns on import. Nowmodel_config = ConfigDict(arbitrary_types_allowed=True).Retry-After. AddsRateLimitError(RoeAPIException)(existingexcept RoeAPIExceptionstill matches), and the transport now waits for an integerRetry-Afteron 429/503, capped at 60s.Testing
test_policies_wrapper_transport.py,test_translate_response.py,test_file_upload.py,test_transport.py).uv run pytest,ruff check,ruff format --check. Merges cleanly with Fix max_retries=0, wait timeouts, run_many retries and partial failures, input encoding #89 and Fix batch examples reading .outputs from batch results #90.Fixes #88.
Note: a Roe API key is required to test these changes end to end; the unit tests here run without one.