Fix Critical Bugs and Refactor Tests - #26
Merged
jaleman-vdr-wikimedia merged 1 commit intoMar 24, 2026
Merged
Conversation
This PR introduces fixes to the SDK that target out of memory vulnerabilities when working with massive data dumps. It introduces thread-safe concurrent downloads and improves developer experience. Alongside these changes, the test suite
was changed accordingly to test for these new changes on the SDK. Below are the changes in more granularity:
1) Deprecated eager memory loading (response.content). Replaced with iter_lines() and iter_bytes() to ensure the SDK maintains a near-zero memory footprint, even when processing multi-gigabyte Wikipedia snapshots.
2) Completely removed the highly volatile _TarfileStreamWrapper. Compressed .tar.gz streams are now securely diverted to temporary disk storage using tempfile.mkstemp and rigorously cleaned up via a finally block, preventing byte-stealing and decompression crashes.
3) Introduced threading.Lock() inside the ThreadPoolExecutor to ensure concurrent byte chunks write to the BytesIO buffer safely without race conditions or silent data corruption.
4) Implemented threading.Event() to manage thread lifecycles. If a single download chunk fails due to a network error, the event is set, instantly aborting all remaining parallel downloads to save CPU cycles and bandwidth.
5) Fixed an issue where complex Request filters were being passed as params to a streaming GET request (which risks being stripped by proxies/HTTP clients). The _subscribe_to_entity method now correctly uses POST with a structured json= body, perfectly aligning with WME API docs.
6) Added explicit string casting (str(k)) for partition integers inside the Request.to_json() builder (since_per_partition). This prevents downstream TypeError crashes when interfacing with strict JSON serializers.
7) Increased the default download_chunk_size from -1 to a safe 20MB buffer to prevent load-balancer timeouts on massive file downloads.
8) Updated all public-facing methods (e.g., get_project, read_batch) so the req: Request parameter is now Optional[Request] = None. Users can now make simple, single-line queries without being forced to import and instantiate empty Request payloads.
9) Implemented a mechanism to capture and manually trigger dynamically generated inner functions (parsing_callback). This allows to successfully test deeply nested except DataModelError: blocks that were previously unreachable.
10) Updated dictionary assertions to expect type-casted strings ('0' instead of 0) and modified stream assertions to expect POST with json= kwargs.
11) Replaced global threading.Event mocks with test_download_chunk_aborts_gracefully. The test now forces one chunk to download slowly while another crashes, to verify that the SDK's thread locks and abort switches behave under network stress.
12) Added tests that dynamically generate actual compressed .tar.gz and .gz archives in memory, but inject them with raw text, malformed NDJSON, and garbage bytes. This proves the SDK gracefully skips bad lines and catches tarfile.TarError and gzip.BadGzipFile exceptions without crashing.
13) Updated test_read_loop to accurately mock httpx.Response.iter_lines() instead of feeding raw BytesIO objects, accurately reflecting the SDK's new low RAM streaming architecture.
14) Deleted TestTarfileStreamWrapper class, as the volatile wrapper itself was removed from the core SDK in favor of safe disk extraction.
15) Removed duplicate exception-checking loops (e.g., test_get_entity_methods_raise_apidataerror...) in favor of a single, global mock that dynamically cycles through API methods.
16) Replaced flaky datetime.now() calls with hardcoded timestamps to prevent random test failures caused by millisecond rollovers.
jaleman-vdr-wikimedia
merged commit Mar 24, 2026
2f5fb36
into
wikimedia-enterprise:main
1 check passed
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.
This PR introduces fixes to the SDK that target out of memory vulnerabilities when working with massive data dumps. It introduces thread-safe concurrent downloads and improves developer experience. Alongside these changes, the test suite was changed accordingly to test for these new changes on the SDK. Below are the changes in more granularity:
Deprecated eager memory loading (response.content). Replaced with iter_lines() and iter_bytes() to ensure the SDK maintains a near-zero memory footprint, even when processing multi-gigabyte Wikipedia snapshots.
Completely removed the highly volatile _TarfileStreamWrapper. Compressed .tar.gz streams are now securely diverted to temporary disk storage using tempfile.mkstemp and rigorously cleaned up via a finally block, preventing byte-stealing and decompression crashes.
Introduced threading.Lock() inside the ThreadPoolExecutor to ensure concurrent byte chunks write to the BytesIO buffer safely without race conditions or silent data corruption.
Implemented threading.Event() to manage thread lifecycles. If a single download chunk fails due to a network error, the event is set, instantly aborting all remaining parallel downloads to save CPU cycles and bandwidth.
Fixed an issue where complex Request filters were being passed as params to a streaming GET request (which risks being stripped by proxies/HTTP clients). The _subscribe_to_entity method now correctly uses POST with a structured json= body, perfectly aligning with WME API docs.
Added explicit string casting (str(k)) for partition integers inside the Request.to_json() builder (since_per_partition). This prevents downstream TypeError crashes when interfacing with strict JSON serializers.
Increased the default download_chunk_size from -1 to a safe 20MB buffer to prevent load-balancer timeouts on massive file downloads.
Updated all public-facing methods (e.g., get_project, read_batch) so the req: Request parameter is now Optional[Request] = None. Users can now make simple, single-line queries without being forced to import and instantiate empty Request payloads.
Implemented a mechanism to capture and manually trigger dynamically generated inner functions (parsing_callback). This allows to successfully test deeply nested except DataModelError: blocks that were previously unreachable.
Updated dictionary assertions to expect type-casted strings ('0' instead of 0) and modified stream assertions to expect POST with json= kwargs.
Replaced global threading.Event mocks with test_download_chunk_aborts_gracefully. The test now forces one chunk to download slowly while another crashes, to verify that the SDK's thread locks and abort switches behave under network stress.
Added tests that dynamically generate actual compressed .tar.gz and .gz archives in memory, but inject them with raw text, malformed NDJSON, and garbage bytes. This proves the SDK gracefully skips bad lines and catches tarfile.TarError and gzip.BadGzipFile exceptions without crashing.
Updated test_read_loop to accurately mock httpx.Response.iter_lines() instead of feeding raw BytesIO objects, accurately reflecting the SDK's new low RAM streaming architecture.
Deleted TestTarfileStreamWrapper class, as the volatile wrapper itself was removed from the core SDK in favor of safe disk extraction.
Removed duplicate exception-checking loops (e.g., test_get_entity_methods_raise_apidataerror...) in favor of a single, global mock that dynamically cycles through API methods.
Replaced flaky datetime.now() calls with hardcoded timestamps to prevent random test failures caused by millisecond rollovers.