-
Notifications
You must be signed in to change notification settings - Fork 49
feat(file-based): throttle legacy per-slice state emission by default #1104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Anatolii Yatsuk (tolik0)
wants to merge
10
commits into
main
Choose a base branch
from
tolik0/cdk/throttle-legacy-state-emission
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
01d0358
feat(streams): time-throttle per-slice state emission via opt-in stre…
tolik0 d229da6
feat(file-based): propagate state-emission throttle from FileBasedSou…
tolik0 79f1394
fix(streams): always emit the cold-start checkpoint, measure throttle…
tolik0 a9bd031
fix(file-based): apply the state throttle where connectors cannot byp…
tolik0 5a99392
test(streams): assert full state cursor sequence; drop dead assignment
tolik0 47bf615
docs(streams): note that a non-positive throttle fails open
tolik0 e0668aa
feat(file-based): throttle legacy per-slice state emission by default
tolik0 1c13b8e
refactor(file-based): move the state throttle out of Stream.read()
tolik0 fc0caf3
fix(streams): share the throttle window check between both paths
tolik0 769753f
fix(streams): honour the fail-open contract under a clock rollback
tolik0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
106 changes: 106 additions & 0 deletions
106
unit_tests/sources/file_based/stream/test_state_emission_throttle.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| # | ||
| # Copyright (c) 2026 Airbyte, Inc., all rights reserved. | ||
| # | ||
| """File-based streams throttle legacy per-slice state emission. | ||
|
|
||
| The wiring lives in `DefaultFileBasedStream._get_checkpoint_reader()` rather than | ||
| in `Stream.read()`, so the generic base class is untouched and only file-based | ||
| streams change behaviour. Because it is a method on the stream class, connectors | ||
| that build their own `DefaultFileBasedStream` subclass from an overridden | ||
| `_make_default_stream()` without calling `super()` — source-s3 and source-gcs | ||
| both do — inherit it and cannot bypass it. | ||
| """ | ||
|
|
||
| import logging | ||
| from typing import Any, Mapping, Optional | ||
| from unittest.mock import MagicMock | ||
|
|
||
| from airbyte_cdk.models import SyncMode | ||
| from airbyte_cdk.sources.file_based.config.csv_format import CsvFormat | ||
| from airbyte_cdk.sources.file_based.config.file_based_stream_config import FileBasedStreamConfig | ||
| from airbyte_cdk.sources.file_based.stream import ( | ||
| DefaultFileBasedStream, | ||
| PermissionsFileBasedStream, | ||
| ) | ||
| from airbyte_cdk.sources.file_based.stream.cursor import DefaultFileBasedCursor | ||
| from airbyte_cdk.sources.streams.checkpoint import ( | ||
| DEFAULT_STATE_EMISSION_THROTTLE_SECONDS, | ||
| ThrottledCheckpointReader, | ||
| ) | ||
|
|
||
|
|
||
| def _config() -> FileBasedStreamConfig: | ||
| return FileBasedStreamConfig(name="stream1", format=CsvFormat(), globs=["*.csv"]) | ||
|
|
||
|
|
||
| def _stream(cls: type = DefaultFileBasedStream, **overrides: Any) -> DefaultFileBasedStream: | ||
| config = _config() | ||
| # No files: `_get_checkpoint_reader` walks `stream_slices()`, and an empty | ||
| # listing is enough to reach the reader construction we care about. | ||
| stream_reader = MagicMock() | ||
| stream_reader.get_matching_files.return_value = [] | ||
| kwargs: Mapping[str, Any] = { | ||
| "config": config, | ||
| "catalog_schema": None, | ||
| "stream_reader": stream_reader, | ||
| "availability_strategy": None, | ||
| "discovery_policy": None, | ||
| "parsers": None, | ||
| "validation_policy": None, | ||
| "errors_collector": None, | ||
| "cursor": DefaultFileBasedCursor(config), | ||
| "use_file_transfer": False, | ||
| "preserve_directory_structure": True, | ||
| **overrides, | ||
| } | ||
| return cls(**kwargs) # type: ignore[arg-type] | ||
|
|
||
|
|
||
| def _reader(stream: DefaultFileBasedStream) -> Any: | ||
| return stream._get_checkpoint_reader( | ||
| logger=logging.getLogger("test"), | ||
| cursor_field=None, | ||
| sync_mode=SyncMode.incremental, | ||
| stream_state={}, | ||
| ) | ||
|
|
||
|
|
||
| def test_checkpoint_reader_is_throttled_at_the_shared_default() -> None: | ||
| reader = _reader(_stream()) | ||
| assert isinstance(reader, ThrottledCheckpointReader) | ||
| assert reader._throttle_seconds == DEFAULT_STATE_EMISSION_THROTTLE_SECONDS | ||
|
|
||
|
|
||
| def test_connector_subclass_cannot_bypass_the_throttle() -> None: | ||
| """The regression this file exists for: source-s3 and source-gcs subclass this | ||
| stream and construct it themselves, so the throttle must ride on the class.""" | ||
|
|
||
| class _ConnectorStream(DefaultFileBasedStream): | ||
| """Mirrors source_gcs.stream.GCSStream / source_s3 ThrottledFileBasedStream.""" | ||
|
|
||
| assert isinstance(_reader(_stream(_ConnectorStream)), ThrottledCheckpointReader) | ||
|
|
||
|
|
||
| def test_permissions_stream_is_throttled_too() -> None: | ||
| """The permissions transfer path is on the same legacy emission path. | ||
|
|
||
| Asserts the constructed reader, not just the class attribute: an override of | ||
| `_get_checkpoint_reader` on this subclass would leave the attribute intact | ||
| while removing the throttle entirely. | ||
| """ | ||
| stream = _stream(PermissionsFileBasedStream, stream_permissions_reader=MagicMock()) | ||
| assert isinstance(_reader(stream), ThrottledCheckpointReader) | ||
| assert ( | ||
| PermissionsFileBasedStream.state_emission_throttle_seconds | ||
| == DEFAULT_STATE_EMISSION_THROTTLE_SECONDS | ||
| ) | ||
|
|
||
|
|
||
| def test_setting_the_throttle_to_none_restores_the_unwrapped_reader() -> None: | ||
| """An escape hatch that returns the base reader untouched, so the throttle can | ||
| be turned off without a different code path.""" | ||
|
|
||
| class _UnthrottledStream(DefaultFileBasedStream): | ||
| state_emission_throttle_seconds: Optional[float] = None | ||
|
|
||
| assert not isinstance(_reader(_stream(_UnthrottledStream)), ThrottledCheckpointReader) | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.