feat: output.get_download_url() + SDK User-Agent identification#13
Conversation
Add a first-class Output.get_download_url() returning {url, expires_at} — a
directly-fetchable URL for an output (signed URL on object-storage backends,
content URL on self-hosted; never throws). Also set a User-Agent identifying
the SDK on every request, with an optional client_info app token.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Warning
|
| Layer / File(s) | Summary |
|---|---|
Client identity headers src/comfy_low/transport.py, src/comfy_sdk/client.py, README.md, tests/conftest.py, tests/test_user_agent.py |
Clients construct validated SDK User-Agent headers, forward client_info, document the behavior, and test default, custom, and rejected values. |
Download URL resolution
| Layer / File(s) | Summary |
|---|---|
Download URL resolution src/comfy_low/transport.py, src/comfy_sdk/outputs.py, src/comfy_sdk/__init__.py, tests/conftest.py, tests/test_get_download_url.py |
Sync and async transports resolve redirect or inline asset URLs, parse signed URL expiry, expose DownloadUrl, and test multi-output and redirect handling. |
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/output-get-download-url
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
feat/output-get-download-url
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/comfy_low/transport.py (1)
125-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor caller
User-Agentoverrides case-insensitively.
{"user-agent": "custom"}leaves bothUser-Agentanduser-agentin the mapping, so precedence becomes transport/server-dependent. Normalize the caller value by casefold before merging, and add a regression test for lowercaseuser-agent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/comfy_low/transport.py` around lines 125 - 126, Update the header merge logic around the extra-header handling to detect and normalize caller-provided User-Agent keys case-insensitively before merging, ensuring the caller’s value is the sole effective User-Agent. Add a regression test covering lowercase “user-agent” input and verify it overrides the transport default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/comfy_low/transport.py`:
- Around line 155-160: Update the expiry parsing flow around the datetime
calculation and X-Goog-Expires conversion to catch OverflowError from timedelta
or datetime addition, returning None for overflowing values just like other
malformed parameters. Preserve the existing successful return path for valid
expiry values.
---
Outside diff comments:
In `@src/comfy_low/transport.py`:
- Around line 125-126: Update the header merge logic around the extra-header
handling to detect and normalize caller-provided User-Agent keys
case-insensitively before merging, ensuring the caller’s value is the sole
effective User-Agent. Add a regression test covering lowercase “user-agent”
input and verify it overrides the transport default.
🪄 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 Plus
Run ID: 0e8294ad-99b0-41d8-a336-adbdb4c0a39c
📒 Files selected for processing (8)
README.mdsrc/comfy_low/transport.pysrc/comfy_sdk/__init__.pysrc/comfy_sdk/client.pysrc/comfy_sdk/outputs.pytests/conftest.pytests/test_get_download_url.pytests/test_user_agent.py
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
What
Output.get_download_url()(sync + async) →DownloadUrl{ url, expires_at }: a directly-fetchable URL for an output, so a caller (e.g. a serverless worker) can hand the URL to a downstream consumer without streaming the bytes through itself.Location) + its expiry.expires_at=None.User-Agentidentification on every request:comfy-sdk-python/{version} ({runtime}), with an optionalclient_info="my-app"appending anapp/my-apptoken so an integration can attribute its own traffic. Request metadata only — not telemetry, no phone-home; a caller-suppliedUser-Agentstill wins.How
get_asset_content_url()issuesGET /assets/{id}/contentwith a per-requestfollow_redirects=False(the shared client default is untouched), reads the redirectLocationwithout following it (so the bearer can't leak cross-origin), and parses the GCS V4X-Goog-Date/X-Goog-Expiresparams for the expiry.Tests
pytest— 90 passed (incl. cloud-redirect, self-hosted-inline, multi-output-distinct-URLs, bearer-not-leaked, User-Agent format +client_infoappend).ruff+mypyclean. No OpenAPI/contract change (sameGET /assets/{id}/contentoperation).