fix: avoid mutating the caller's common_attributes in timestream.write - #3439
Open
hsusul wants to merge 1 commit into
Open
fix: avoid mutating the caller's common_attributes in timestream.write#3439hsusul wants to merge 1 commit into
hsusul wants to merge 1 commit into
Conversation
`_sanitize_common_attributes` filled in the `Version`, `TimeUnit` and `MeasureName` defaults with `setdefault` directly on the dictionary passed by the caller. Reusing that dictionary for a later `wr.timestream.write` call silently pinned the first call's values, so a subsequent call's `version`, `time_unit` and `measure_name`/`measure_col` arguments were ignored -- writing records under the wrong measure name, at the wrong version, and with timestamps formatted in the wrong time unit. Copy the dictionary before resolving defaults. Precedence is unchanged: values supplied in `common_attributes` still win over the other arguments.
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.
Feature or Bugfix
Detail
wr.timestream.writemutates thecommon_attributesdictionary supplied by the caller. Reusing that dictionary for a later write silently discards the later call'sversion,time_unitandmeasure_name/measure_colarguments.Affected API:
awswrangler.timestream.write(viaawswrangler/timestream/_write.py::_sanitize_common_attributes)Root cause
_sanitize_common_attributesresolves its defaults withsetdefaulton the object it was handed, not on a copy:After one
write, the caller's dictionary permanently carriesVersion,TimeUnitandMeasureName. Becausecommon_attributestakes precedence over the other arguments by design, every subsequentwritereusing that dictionary is pinned to the first call's values.Reproduction (no AWS credentials — moto only)
Current behavior
The caller's dictionary has been modified, and the second write ignores
measure_col="mem",version=5andtime_unit="MICROSECONDS". Thememvalues are stored under the measure namecpu, at version 1 (so a genuine upsert is silently rejected), and the timestamp is emitted in milliseconds while the request declares nothing to the contrary — a 1000x error against the requested unit.Corrected behavior
Implementation
One-line change in
_sanitize_common_attributes: copy the dictionary before resolving defaults.A shallow copy is sufficient — only top-level keys are written; nested values such as
Dimensionsare read, never modified. Precedence semantics, validation and error behavior are unchanged, and no public API changed.Regression tests
Added to
tests/unit/test_moto.py(the module that runs in the PR test job), using a newmoto_timestream_sessionfixture. No AWS credentials required.test_timestream_write_does_not_mutate_common_attributes— the caller's dictionary is byte-for-byte unchanged after a successful write.test_timestream_write_common_attributes_reused_across_calls— reusing one dictionary across two writes: the second call'smeasure_col,versionandtime_unitare honored, asserted on the capturedWriteRecordsparameters including the formatted epoch value (timezone-aware input, so the assertion is independent of the local timezone).test_timestream_write_common_attributes_take_precedence— guard that the copy did not change precedence:MeasureName,TimeUnitandVersionsupplied insidecommon_attributesstill win overmeasure_name,time_unitandversion.The first two fail on current
mainand pass with the fix; the third passes both ways.Validation (macOS, Python 3.13.5, moto 5.2.2, boto3 1.42.68)
Run exactly as in
.github/workflows/minimal-tests.yml:pytest tests/unit/test_metadata.py tests/unit/test_session.py tests/unit/test_utils.py— 20 passedpytest -n 4 tests/unit/test_moto.py— 49 passedruff format --check .— 280 files already formattedruff check .— all checks passedmypy awswrangler— 19 errors, identical to the pre-existing count onmain; none inawswrangler/timestreamdoc8 --max-line-length 120 docs/source— exit 0uv lock --check— up to dategit diff --check— cleanNot run: the AWS integration suites (
tests/unit/test_timestream.pyand the othertests/unit/*modules), which require a live AWS account and thetest_infraCDK stacks. The behavior they would cover is exercised locally through moto and through assertions on the exactWriteRecordsrequest parameters.Compatibility
No public API, signature, default or dtype change. Callers that relied on reading back the resolved defaults from their own
common_attributesdictionary after a write — undocumented behavior — would no longer see them; the resolved values are still sent in the request as before.Relates
awswrangler/timestream/_write.py.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.