Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1060 +/- ##
==========================================
- Coverage 95.24% 95.21% -0.03%
==========================================
Files 59 60 +1
Lines 5509 5669 +160
==========================================
+ Hits 5247 5398 +151
- Misses 262 271 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| if isinstance(data, StreamedRequestBody): | ||
| return (headers, self._parse_params(params), data) | ||
|
|
||
| if StreamedRequestBody.is_source(data): |
There was a problem hiding this comment.
StreamedRequestBody.is_source(data) reads more like is data an existing source of StreamedRequestBody
Maybe it should have a name that implies that the method is testing a possibility, not the existing reality.
I am bad with naming, but just to give a hint in which direction I think it should go
StreamedRequestBody.is_possible_source(data)
StreamedRequestBody.is_compatible_source(data)
StreamedRequestBody.can_use_as_source(data)
...
| pipeline untouched. The constructor refuses one, so a caller that builds a body from what this accepts has | ||
| to check for an existing body first. | ||
| """ | ||
| return ( |
There was a problem hiding this comment.
This seems too restrictive for all iterables. (We want to keep some special iterables - like strings, lists, ... out of streaming, but is there really a reason to keep all of them out?l)
For example (simplified example just to demonstrate the point)
class Chunked:
"""Wraps a string and yields it in pieces. Only __iter__, nothing else."""
def __init__(self, text: str, size: int = 10) -> None:
self.text, self.size = text, size
def __iter__(self):
for i in range(0, len(self.text), self.size):
yield self.text[i : i + self.size]
client.actor('E2jjCZBezvAZnX8Rb').start(
run_input=Chunked('{"message": "Hello world!"}'),
content_type='application/json',
)
On your PR it is serialized as
"<main.Chunked object at 0x7f97fb28a8d0>"
I asked to create a commit with an example implementation that would also support these:
7b34e44
| self._start: int | None = None | ||
|
|
||
| # A response is recognized first, so its `read` is never touched - on an unread streaming response that | ||
| # either raises or buffers the whole body. |
There was a problem hiding this comment.
It does not feel very robust.
The detection in StreamedRequestBody is a chain of getattr probes. That works for the standard library, but this code takes arbitrary user objects, and for those the outcome depends on which method names they happen to have.
_accepts_chunk_size is going even further with guessing.
So for user-defined code or libraries we have not considered, this can behave in completely unpredictable ways.
What changed
set_record, therun_inputofstart/call/metamorph, andHttpClient.call(data=...)now accept a file-like object, an iterator ofbytesorstrchunks, or a streamedHttpResponse. The body goes out chunked and uncompressed, so one Actor'sOUTPUTrecord can become another Actor's input without the orchestrating process holding it in memory. The async client also takes async iterators andaiofiles-style objects; the sync client rejects those with aTypeErrornamingApifyClientAsync.A file-like object is read through
read(chunk_size). Areadthat takes no size is called once and its result sent as a single chunk, so a duck-typed reader of that shape keeps working and keeps its old memory profile.Retries
Only a seekable file-like source is retried, sought back to its starting position before each attempt. Anything else gets a single attempt. An error raised inside the body iterator surfaces as itself with the transport error as its cause, since Impit reports it as a bare
impit.HTTPErrorthat the classifier would treat as transient.Compression
A file-like
set_recordvalue is no longer compressed, so uploading from a file handle sends more bytes over the wire, which Apify bills as data transfer. Passingfile.read()keeps the old behavior. Not breaking: the call keeps working, and the extra data transfer belongs in the changelog rather than in a major bump.A streamed value that declares a
content_encodingis taken at its word, since the client only sees its bytes as they go out. Astr, a JSON-serializable object, and a text-mode file are still rejected.Custom transports
send_requestnow receivesbytes | Iterator[bytes] | None(async:AsyncIterator[bytes]), andcallaccepts the new source types. A transport typed againstbytes | Noneonly sees an iterator once one of its own callers opts into streaming, so this isn't breaking either.Docs
A "Streaming uploads" section on the streaming concept page, plus updates to the compression and HTTP client pages, the custom transport examples, and the README. The guide on chaining Actors into a pipeline is a separate PR.
Tests
StreamedRequestBodyon its own, the retry pipeline with fake transports, andset_recordend to end over Impit and HTTPX2. The two integration tests (3 MiB upload, record piped between stores) weren't run locally.Issues
✍️ Drafted by Claude Code