Conversation
WalkthroughThis PR modernizes the TrueFoundry SDK by introducing comprehensive logging infrastructure, systematic response validation error handling via Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AsyncBaseTrueFoundry
participant _make_default_async_client
participant httpx_aiohttp
participant httpx
Client->>AsyncBaseTrueFoundry: __init__(logging=..., async_token=...)
AsyncBaseTrueFoundry->>_make_default_async_client: (when no httpx_client provided)
alt httpx_aiohttp available
_make_default_async_client->>httpx_aiohttp: HttpxAiohttpClient(timeout, follow_redirects)
httpx_aiohttp-->>_make_default_async_client: client instance
else httpx_aiohttp unavailable
_make_default_async_client->>httpx: AsyncClient(timeout, follow_redirects)
httpx-->>_make_default_async_client: client instance
end
_make_default_async_client-->>AsyncBaseTrueFoundry: async_http_client
AsyncBaseTrueFoundry->>AsyncClientWrapper: __init__(logging, async_token, httpx_client)
AsyncClientWrapper-->>Client: ready for requests
sequenceDiagram
participant HTTPClient
participant Request
participant Logger
participant httpx
participant Retry Logic
HTTPClient->>Logger: debug("Outbound request", headers=redacted)
HTTPClient->>httpx: request()
alt Success (2xx/3xx)
httpx-->>HTTPClient: Response
HTTPClient->>Logger: info("Response", status=code)
HTTPClient-->>Request: parsed_response
else ConnectError or RemoteProtocolError
httpx-->>HTTPClient: exception
HTTPClient->>Retry Logic: retries < max_retries?
alt Retries remaining
Retry Logic->>Retry Logic: sleep(exponential_backoff)
Retry Logic->>httpx: request() [recursive]
else No retries left
Retry Logic-->>HTTPClient: re-raise exception
end
else Status >= 400
httpx-->>HTTPClient: Response
HTTPClient->>Logger: error("Response", status=code)
alt ValidationError during parse
HTTPClient-->>Request: ParsingError(status, headers, body, cause)
else JSONDecodeError
HTTPClient-->>Request: ApiError
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/truefoundry_sdk/application_versions/raw_client.py`:
- Line 68: The public raw client is percent-encoding path parameters (using
encode_path_param) when constructing URLs like
"api/svc/v1/apps/{encode_path_param(id)}/deployments" which will double-encode
inputs already escaped by callers; remove the encode_path_param calls from the
public client URL construction so IDs and deployment_id are used verbatim (e.g.,
change occurrences of encode_path_param(id) and encode_path_param(deployment_id)
in src/truefoundry_sdk/application_versions/raw_client.py to the raw parameter)
or alternatively add a clear migration note if you intend this breaking
change—update the URL-building sites referenced (the lines using
encode_path_param in the deployments and app-specific endpoint builders)
accordingly.
In `@src/truefoundry_sdk/data_directories/raw_client.py`:
- Around line 83-86: The change wraps pydantic.ValidationError into a custom
ParsingError (via the except block that catches ValidationError and raises
ParsingError), which breaks the public exception contract; revert this by
preserving the original ValidationError surface: in the except ValidationError
handlers around parse_obj_as(...) in raw_client.py (the blocks that currently
raise ParsingError) stop converting to ParsingError and re-raise the caught
ValidationError (or raise a new ValidationError enriched with response metadata)
so callers still receive pydantic.ValidationError; apply the same fix to the
other matching handlers mentioned (the block around lines 658-661).
In `@src/truefoundry_sdk/prompts/raw_client.py`:
- Around line 79-82: The diff changes public methods to convert a caught
ValidationError into a ParsingError, breaking callers that catch
ValidationError; revert this by preserving the original ValidationError for the
public raw client methods (replace the current except ValidationError: raise
ParsingError(...) behavior), i.e. when parsing fails in the sync/async blocks
referenced (symbols: ValidationError, ParsingError, _response, cause) re-raise
the original ValidationError (or raise a new ValidationError that includes
response context via chaining) so existing except ValidationError handlers
continue to work; apply this fix to all the corresponding blocks mentioned
(around the ValidationError handlers at the listed locations).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 409e1db3-284b-4cf2-bcd7-4063001edfdc
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.fern/metadata.jsonREADME.mdpyproject.tomlreference.mdrequirements.txtsrc/truefoundry_sdk/__init__.pysrc/truefoundry_sdk/_default_clients.pysrc/truefoundry_sdk/agent_skill_versions/raw_client.pysrc/truefoundry_sdk/agent_skills/raw_client.pysrc/truefoundry_sdk/alerts/raw_client.pysrc/truefoundry_sdk/application_versions/raw_client.pysrc/truefoundry_sdk/applications/raw_client.pysrc/truefoundry_sdk/artifact_versions/raw_client.pysrc/truefoundry_sdk/artifacts/raw_client.pysrc/truefoundry_sdk/base_client.pysrc/truefoundry_sdk/clusters/raw_client.pysrc/truefoundry_sdk/core/__init__.pysrc/truefoundry_sdk/core/client_wrapper.pysrc/truefoundry_sdk/core/custom_pagination.pysrc/truefoundry_sdk/core/datetime_utils.pysrc/truefoundry_sdk/core/http_client.pysrc/truefoundry_sdk/core/jsonable_encoder.pysrc/truefoundry_sdk/core/logging.pysrc/truefoundry_sdk/core/parse_error.pysrc/truefoundry_sdk/core/pydantic_utilities.pysrc/truefoundry_sdk/data_directories/raw_client.pysrc/truefoundry_sdk/environments/raw_client.pysrc/truefoundry_sdk/events/raw_client.pysrc/truefoundry_sdk/internal/ai_gateway/raw_client.pysrc/truefoundry_sdk/internal/applications/raw_client.pysrc/truefoundry_sdk/internal/artifact_versions/raw_client.pysrc/truefoundry_sdk/internal/build_logs/raw_client.pysrc/truefoundry_sdk/internal/clusters/raw_client.pysrc/truefoundry_sdk/internal/deployments/raw_client.pysrc/truefoundry_sdk/internal/docker_registries/raw_client.pysrc/truefoundry_sdk/internal/metrics/raw_client.pysrc/truefoundry_sdk/internal/ml/raw_client.pysrc/truefoundry_sdk/internal/raw_client.pysrc/truefoundry_sdk/internal/users/raw_client.pysrc/truefoundry_sdk/internal/vcs/raw_client.pysrc/truefoundry_sdk/internal/workflows/raw_client.pysrc/truefoundry_sdk/jobs/raw_client.pysrc/truefoundry_sdk/logs/raw_client.pysrc/truefoundry_sdk/ml_repos/raw_client.pysrc/truefoundry_sdk/model_versions/raw_client.pysrc/truefoundry_sdk/models/raw_client.pysrc/truefoundry_sdk/personal_access_tokens/raw_client.pysrc/truefoundry_sdk/prompt_versions/raw_client.pysrc/truefoundry_sdk/prompts/raw_client.pysrc/truefoundry_sdk/raw_base_client.pysrc/truefoundry_sdk/secret_groups/raw_client.pysrc/truefoundry_sdk/secrets/raw_client.pysrc/truefoundry_sdk/teams/raw_client.pysrc/truefoundry_sdk/traces/raw_client.pysrc/truefoundry_sdk/types/non_negative_float.pysrc/truefoundry_sdk/users/raw_client.pysrc/truefoundry_sdk/virtual_accounts/raw_client.pysrc/truefoundry_sdk/workspaces/raw_client.pytests/conftest.pytests/test_aiohttp_autodetect.pytests/utils/test_http_client.py
💤 Files with no reviewable changes (1)
- src/truefoundry_sdk/core/custom_pagination.py
|
|
||
| _response = self._client_wrapper.httpx_client.request( | ||
| f"api/svc/v1/apps/{jsonable_encoder(id)}/deployments", | ||
| f"api/svc/v1/apps/{encode_path_param(id)}/deployments", |
There was a problem hiding this comment.
Percent-encoding path params changes what public callers can pass here.
These public methods now encode id and deployment_id before building the URL. Callers that already pass escaped identifiers into the SDK will now get double-encoding (%2F → %252F), so this needs a migration note or another compatibility plan if this release is meant to stay non-breaking.
Based on learnings, for autogenerated SDK regeneration PRs in truefoundry/truefoundry-python-sdk, chiragjn wants only interface-breaking issues called out on the public client.v1 surface.
Also applies to: 152-152, 243-243, 330-330
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/truefoundry_sdk/application_versions/raw_client.py` at line 68, The
public raw client is percent-encoding path parameters (using encode_path_param)
when constructing URLs like
"api/svc/v1/apps/{encode_path_param(id)}/deployments" which will double-encode
inputs already escaped by callers; remove the encode_path_param calls from the
public client URL construction so IDs and deployment_id are used verbatim (e.g.,
change occurrences of encode_path_param(id) and encode_path_param(deployment_id)
in src/truefoundry_sdk/application_versions/raw_client.py to the raw parameter)
or alternatively add a clear migration note if you intend this breaking
change—update the URL-building sites referenced (the lines using
encode_path_param in the deployments and app-specific endpoint builders)
accordingly.
| except ValidationError as e: | ||
| raise ParsingError( | ||
| status_code=_response.status_code, headers=dict(_response.headers), body=_response.json(), cause=e | ||
| ) |
There was a problem hiding this comment.
Wrapping response-schema mismatches in ParsingError is a breaking exception-contract change.
Before this regen, parse_obj_as(...) validation failures from these public raw-client methods bubbled out as pydantic.ValidationError; now they raise ParsingError instead. That breaks callers that currently catch ValidationError for response drift, so this either needs to preserve the old exception surface or be treated/documented as an explicit breaking change.
Based on learnings, for autogenerated SDK regeneration PRs in truefoundry/truefoundry-python-sdk, chiragjn wants only interface-breaking issues called out on the public client.v1 surface.
Also applies to: 658-661
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 84-86: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/truefoundry_sdk/data_directories/raw_client.py` around lines 83 - 86, The
change wraps pydantic.ValidationError into a custom ParsingError (via the except
block that catches ValidationError and raises ParsingError), which breaks the
public exception contract; revert this by preserving the original
ValidationError surface: in the except ValidationError handlers around
parse_obj_as(...) in raw_client.py (the blocks that currently raise
ParsingError) stop converting to ParsingError and re-raise the caught
ValidationError (or raise a new ValidationError enriched with response metadata)
so callers still receive pydantic.ValidationError; apply the same fix to the
other matching handlers mentioned (the block around lines 658-661).
| except ValidationError as e: | ||
| raise ParsingError( | ||
| status_code=_response.status_code, headers=dict(_response.headers), body=_response.json(), cause=e | ||
| ) |
There was a problem hiding this comment.
Breaking exception contract in public prompts raw client methods
Line 79 (and corresponding async/sync blocks) changes response-validation failures from surfacing ValidationError to surfacing ParsingError. That is a caller-visible behavior change and can break existing except ValidationError handling paths.
Please either preserve prior exception behavior for these public methods or explicitly treat this as a breaking SDK change (versioning + release notes/migration note).
Based on learnings: For autogenerated SDK regeneration PRs in truefoundry/truefoundry-python-sdk, only interface-breaking issues should be called out (public client.v1 surface, removed/renamed types/enums/fields, and behavior that breaks callers).
Also applies to: 133-135, 231-233, 294-296, 352-354, 405-407, 506-508, 569-571
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 80-82: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/truefoundry_sdk/prompts/raw_client.py` around lines 79 - 82, The diff
changes public methods to convert a caught ValidationError into a ParsingError,
breaking callers that catch ValidationError; revert this by preserving the
original ValidationError for the public raw client methods (replace the current
except ValidationError: raise ParsingError(...) behavior), i.e. when parsing
fails in the sync/async blocks referenced (symbols: ValidationError,
ParsingError, _response, cause) re-raise the original ValidationError (or raise
a new ValidationError that includes response context via chaining) so existing
except ValidationError handlers continue to work; apply this fix to all the
corresponding blocks mentioned (around the ValidationError handlers at the
listed locations).
| except ValidationError as e: | ||
| raise ParsingError( | ||
| status_code=_response.status_code, headers=dict(_response.headers), body=_response.json(), cause=e | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== parse_error.py ==="
fd '^parse_error\.py$' src/truefoundry_sdk/core -x sed -n '1,160p' {}
echo
echo "=== api_error.py ==="
fd '^api_error\.py$' src/truefoundry_sdk/core -x sed -n '1,160p' {}
echo
echo "=== validation-remap sites ==="
rg -n -C2 'except ValidationError as e|raise ParsingError' src/truefoundry_sdkRepository: truefoundry/truefoundry-python-sdk
Length of output: 50391
This exception-contract change breaks existing error handlers.
The regeneration remaps pydantic.ValidationError to ParsingError across all public client methods. Since ParsingError does not inherit from ApiError or from pydantic.ValidationError, existing callers with except ValidationError or except ApiError handlers will no longer catch response-schema failures. Either make ParsingError inherit from the appropriate base exception or document this as an explicit breaking change.
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 118-120: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
1 participant
Note
Low Risk
Changes are limited to SDK generation metadata, docs, and dependency/lockfile updates with no direct runtime logic modifications in the source tree.
Overview
Updates Fern Python SDK generator metadata (
generatorVersionto5.5.0) and adjusts generation config to apply request defaults.Refreshes the README examples (token placeholder, import formatting, and pagination snippets) to match the current
applications.listusage.Updates packaging: adds optional
aiohttp/httpx-aiohttpextra (with pytest marker) and constrainspydantic-corewith an upper bound; regeneratespoetry.lockaccordingly.Reviewed by Cursor Bugbot for commit a19685a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Dependencies
aiohttpandhttpx-aiohttppackages.pydantic-coreversion constraints.