feat(low-code): stream paginated JSON items with opt-in spool_to_disk and retry-safe downloads - #1176
Draft
ZaneHyattAB wants to merge 16 commits into
Draft
ZaneHyattAB wants to merge 16 commits into
ZaneHyattAB wants to merge 16 commits into
Conversation
…aware body filters, spool-to-disk Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… bodies Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ith streaming decoders Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…nto (py3.10) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1790304599-stream-json-items-spool#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1790304599-stream-json-items-spoolPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
…ient send signatures stable Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ecoders Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…nator opts in Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…d bodies Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…aluation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
3 tasks
This branch has not been deployed
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
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.
Overview
👉 TL;DR: Lets a low-code connector read very large JSON pages (hundreds of MB) without loading the whole page into memory, while keeping today's pagination, retry and error-handling behaviour. Connectors opt in per stream; nothing changes for manifests that don't.
Specifically:
JsonItemsDecodernow works withDefaultPaginator(the parser captures the document remainder forCursorPagination), a new opt-inJsonItemsDecoder.spool_to_diskdownloads the body insideHttpClient._send_with_retrybefore parsing,HttpResponseFilterno longer reads the body of spooled 2xx responses larger than 1 MiB,use_cache+spool_to_diskfails fast at build time, and streamed parsers see transport-decoded bytes (raw.decode_content = True). No breaking changes: every new behaviour is gated onspool_to_disk: trueor onJsonItemsDecoder+DefaultPaginator(a combinationmainrejects at build time) — see Compatibility below.Requested by Zane Hyatt (on-call) for https://github.com/airbytehq/oncall/issues/13554. Draft: opened for review by the CDK owners before the downstream connector change.
Problem
A manifest-only connector using
JsonDecoderon an incremental export endpoint that returns 1000 records per page (~160 MB decoded JSON, ~160 KB per record) was OOM-killed by the 4 GiB memory cgroup on every attempt (kernel:Memory cgroup out of memory: Killed process (python) anon-rss:~4.1 GB). The page is materialised several times on the way to the record queue — raw bytes, theBytesIO/.contentcopy, a stdlib-jsontree built by a body-basederror_message_containsfilter, then theorjsontree — on top of the 10 000-record queue plateau (~1.6–1.9 GB at that record size).JsonItemsDecoder(streamingijson) would avoid the spike but today (a)DefaultPaginatorrejects it, (b) any body-based response filter drains the socket before the decoder runs, (c)requests_cache(auto-enabled on parent streams) buffers the whole body in memory before the decoder sees it, (d) a connection that breaks mid-body is not retried, and (e) it needs aGzipDecoderwrapper forContent-Encoding: gzip.Changes
JsonItemsParser.parse(data, *, on_document_remainder=None): with a callback, oneijsonpass yields the items and builds the remainder document (everything outside the items array, items array left empty, nested keys preserved); without one it ismain's exactijson.items(data, prefix)path.CompositeRawDecoder(capture_document_remainder=False)only installs the callback aftercreate_default_paginatorcallsenable_document_remainder_capture()on the decoder instance it shares with the record selector, so a non-paginatedJsonItemsDecoderis unchanged.CompositeRawDecoderstashes the remainder on therequests.Response;PaginationDecoderDecoratoryields it instead of{}, so{{ response.get("after_url") }}/{{ response.meta.after_cursor }}work unchanged.GzipParserforwards the kwarg. Factory allow-list (_is_supported_parser_for_pagination) acceptsJsonItemsParserdirectly or Gzip-wrapped;CsvParser/JsonLineParserstill rejected.HttpClient._sendmarksstream=Trueresponses (streamed_response.mark_body_streamed; the marker is inert unless something below reads it).HttpResponseFilterskipspredicate/error_message_contains/error_messagebody reads only for a spooled 2xx body > 1 MiB (http_codesstill apply); a spooled body ≤ 1 MiB is read and the spool rewound afterinterpret_response; unspooled streamed responses and all non-2xx handling are byte-identical tomain.create_http_requesterlogs a warning (Builder-visible) when a spooling decoder is combined with body-based filters, explaining the 1 MiB cut-off.JsonItemsDecoder.spool_to_disk(schema + model, defaultfalse, forced off in Connector Builder mode):HttpClient._spool_response_bodycopies the body viaiter_contentinto atempfile.SpooledTemporaryFile(max_size=8 MiB)inside_send, beforeinterpret_response. The flag travels out-of-band as a privaterequest_kwargskey (_SPOOL_RESPONSE_KWARG, set insend_request, popped in_sendbeforesession.send), so_send_with_retry/_sendkeepmain's exact signatures and subclass overrides keep working.ChunkedEncodingError/ConnectionError/ContentDecodingErrorduring the copy setresponse = Noneand go through the existing fallbackRETRY→DefaultBackoffExceptionpath, i.e. the connector's own backoff/max_retries.response.rawbecomesSpooledResponseBody(aBufferedReaderover a py3.10-safeRawIOBaseadapter —fileno()would force rollover); every parser andresponse.content/.json()read from it. Plumbing:HttpRequester.spool_response(declared afterdecoder, so positional construction is unchanged),Decoder.spools_response(),CompositeRawDecoder(spool_response=),HttpClient.send_request(spool_response=)(appended keyword).create_http_requesterraisesValueErrorfor explicituse_cache: true+ a decoder withspool_to_disk: true(requests_cachereads the whole body, leaving nothing to spool);_initialize_cache_for_parent_streamsskips the auto-enable only when the (resolved) retriever decoder is aJsonItemsDecoderwithspool_to_disk: true, directly or wrapped inGzipDecoder(_decoder_spools_to_disk, all four call sites incl. bothStateDelegatingStreambranches). Other streaming decoders keepmain's caching behaviour.CompositeRawDecoderstream path setsresponse.raw.decode_content = Trueso a bare streamed decoder parsesContent-Encoding: gzipbodies;GzipParserstill sniffs magic bytes, so double-gzip payloads keep working.self._logger.debug(..., extra={...})convention):"Spooled response body to disk"(url,status,bytes,seconds) and"Captured pagination document remainder"(keys).Review Spotlight
Reviewers with limited time, please review first:
http_client.py#L430-L505: spool inside the retry loop;response = Noneon copy failure is what makes the retry happen;_content_consumed = Falseis the one privaterequeststouch.streamed_response.py: response markers andSpooledResponseBody.http_response_filter.py#L27-L34: the 1 MiB body-filter gate.composite_raw_decoder.py#L185-L213: single-pass items + remainder.concurrent_declarative_source.py_decoder_spools_to_disk/_set_cache_if_not_disabled: parent auto-cache skip for spooled decoders only.http_client.py_SPOOL_RESPONSE_KWARG: the out-of-band spool flag that keeps_send/_send_with_retrysignatures stable.Gating:
JsonItemsDecoder.spool_to_disk(defaultfalse)Spooling is opt-in per decoder. Without it,
JsonItemsDecoder+DefaultPaginatorstreams straight from the socket (today's semantics for streamed decoders: a mid-body failure is not retried, records already yielded cannot be rolled back — same reasonREDUCE_PAGE_SIZErefuses re-requests).Compatibility
No breaking changes. Every new behaviour is opt-in — it requires either
spool_to_disk: trueorJsonItemsDecoder+DefaultPaginator(whichmainrejects at build time withdecoder not supported for pagination, so no existing manifest has it). Audited against cleanmain(d8d8e6f6) with a local harness (manifests run throughConcurrentDeclarativeSourcein sync and Builder mode against a stub HTTP server, plus parse benchmarks):mainHttpClient._send/_send_with_retrysignatures; subclasses overriding them (e.g.source-amazon-seller-partner'sRateLimitConfigErrorHttpClient)HttpClient.send_request,CompositeRawDecoder.__init__,HttpRequesterfields,DecoderABCdataclasses.fields(HttpRequester)=main+["spool_response"]use_cache: true+ streaming decoder (Csv/Jsonl/Gzip/JsonItems)main(cache miss and hit both return records);ValueErroronly withspool_to_disk: trueCsv/Jsonl/Gzip/JsonItemsparentsmain; skipped only forspool_to_disk: truemain(incl. the body being consumed)http_codesstill apply) + build-time warningJsonItemsDecoderwithout paginatorijson.itemspath; parse time and peak memory matchmainon a 1000 × 300-field page and on a document with a 50 000-element second array (identical record hashes)JsonItemsDecoder(+GzipDecoder) +DefaultPaginatorValueErroronmain→ builds and paginatesPaginationDecoderDecoratorforJson/Xml/Gzip(Json)/Jsonl/Csvspool_to_diskaccepted and ignored; new logs are DEBUG (plus the opt-in WARNING above)JsonItemsDecoder(defaultfalse); old manifests validate unchanged, noversionbump or migrationContent-Encoding: gzip/deflate(raw.decode_content = True)mainyields identical records here; the 9 cells that raised onmain(JsonItems/Csvon gzip/deflate transport,Gzip(*)on deflate) now parse.GzipParsersniffs magic bytes, so double-gzip payloads still work.brnot measured (module absent).HttpStreamconnectors passingstream=TrueHttpResponseFilter/PaginationDecoderDecoratorCorrection to an earlier claim in this description:
requests_cache(1.2.1) does not drain a streamed body —_copy_bodyreads it and_reset_fpre-armsresponse.raw, souse_cache+ streaming decoder returns records onmain(at the cost of holding the whole body in memory). That is why the cache is now only refused forspool_to_disk, where memory is the point.Benchmarks
Real
HttpClient+ sharedDefaultErrorHandler(witherror_message_contains) + decoder +PaginationDecoderDecorator, synthetic 1000 × 160 KB gzip page, fresh subprocess,ru_maxrss:JsonDecoder(today)JsonItemsDecoder,spool_to_diskspool_to_disk, consumer at 4 rec/sspool_to_disk, connection cut at 60 %Unspooled
JsonItemsDecoderwith this connector's body-based error handler now behaves as onmain(the filter consumes the stream →IncompleteJSONError), sospool_to_diskis the pairing to use there.Queue plateau (not changed here): 10 000 parsed records cost ≈ 1.1–1.25 B per JSON byte, i.e. ~1.65–1.9 GB at 160 KB/record.
Risks / remaining tradeoffs
TMPDIR; logged per page at debug).main(filter consumes the stream).response._content_consumed = False(privaterequestsattribute, single site, unit-tested) and a hand-editeddeclarative_component_schema.py(+5 lines, matching the approach in feat: add REDUCE_PAGE_SIZE response action for dynamic page-size reduction #1149 because the generator's output currently fails mypy).Follow-ups (separate PRs)
Queue(maxsize=10_000)inconcurrent_declarative_source.pyis count-based).HttpComponentsResolversetsuse_cache=Trueunconditionally; aspool_to_diskdecoder there would hit theValueError.spool_to_diskforJsonlDecoder/CsvDecoder/GzipDecoder.source-zendesk-supporttickets_streamgets 8 manifest lines —decoder: {type: JsonItemsDecoder, items_path: tickets, spool_to_disk: true}andrecord_selector.extractor: {type: DpathExtractor, field_path: []}— plus the base-image bump; nouse_cacheline, nocomponents.py, shared requester/paginator/error handler untouched. The connector's full unit suite (231 tests) passes against this branch with that manifest.Test plan
HttpClientsubclass overriding_send_with_retrywithmain's exact signature (mirrorssource-amazon-seller-partner) with and without spooling;use_cache: true+JsonlDecoderbuilds / +spool_to_diskraises;Csv/Jsonl/Gzip/JsonItemsparents get auto-cache, spooled andGzip(spooled)parents (incl.StateDelegatingStreambranch) don't; remainder capture off by default and enabled bycreate_default_paginator; positionalHttpRequesterconstruction inmain's order; unspooled streamed 2xx evaluates body filters; build-time warning presence/absence.CursorPaginationend-to-end; filter marker/size semantics (skip on unspooled/large, evaluate on ≤ 1 MiB,http_codesfirst, non-2xx unchanged); spool: gzip decode, rewind afterinterpret_response,ProtocolError/DecodeError/ReadTimeoutErroreach → 2 sends and a good response, all five parsers × {in-memory, rolled-over}; factory guard/propagation/Builder mode; parent auto-cache skip incl. mixedStateDelegatingStream.poetry run pytestonunit_tests/sources/declarative/{decoders,requesters,retrievers,parsers},unit_tests/sources/streams/http,test_concurrent_declarative_source*: 1606 passed, 2 failed, 1 skipped. The 2 failures (test_read_with_concurrent_and_synchronous_streams_with_{concurrent,sequential}_state,sqlite3 database table is lockedinrequests_cache) fail identically on a cleanmain(d8d8e6f6) checkout — pre-existing.poetry run ruff check .,poetry run ruff format --check .,poetry run mypy --config-file mypy.ini airbyte_cdk: clean.SpooledTemporaryFile.readintodoes not exist there): 175 passed.source-zendesk-supportfullunit_testsagainst this branch with the target manifest: 231 passed.Link to Devin session: https://app.devin.ai/sessions/9fb9d3e97c71470bbbd2b6f242fb725e
Open in Devin Desktop: https://app.devin.ai/desktop/session/9fb9d3e97c71470bbbd2b6f242fb725e?variant=devin
Requested by: ZaneHyattAB