Sum DynamoDB ConsumedCapacity across paginated pages (CLI-4199) - #10656
GarrettBeatty wants to merge 9 commits into
Conversation
aws dynamodb scan/query with auto-pagination previously reported only the final page's ConsumedCapacity, undercounting the true consumed capacity of the operation (CLI-4199). Add an opt-in "aggregate_numeric_keys" paginator directive that recursively sums the numeric leaves of a response member across pages, preserving strings (e.g. TableName) and handling maps keyed by runtime-defined names such as GlobalSecondaryIndexes/LocalSecondaryIndexes. Declare ConsumedCapacity under this directive for Scan and Query. The directive is opt-in per paginator config, so no other service's pagination behavior changes.
paginators-1.json is regenerated from upstream models, so hand edits there are clobbered on the next sync. Move the ConsumedCapacity aggregation config into a paginators-1.sdk-extras.json overlay (deep-merged at load time), matching the convention used across other services. This restores the generated paginators-1.json to its original state and durably (a) clears the single-page non_aggregate_keys entry and (b) declares aggregate_numeric_keys.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Fixes DynamoDB auto-pagination reporting by summing ConsumedCapacity across all paginated pages (CLI-4199), instead of only using the last page’s value.
Changes:
- Added an opt-in paginator directive
aggregate_numeric_keysand implemented recursive numeric-leaf aggregation inbuild_full_result. - Updated DynamoDB paginator extras so
Scan/QueryaggregateConsumedCapacitycorrectly. - Added unit + functional tests and updated the paginator config linter to recognize the new directive.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| awscli/botocore/paginate.py | Implements _deep_add_numeric + new aggregate_numeric_keys aggregation behavior in build_full_result. |
| awscli/botocore/data/dynamodb/2012-08-10/paginators-1.sdk-extras.json | Opts DynamoDB Scan/Query into aggregating ConsumedCapacity. |
| tests/unit/botocore/test_paginate.py | Adds unit tests for _deep_add_numeric and aggregation behavior across pages. |
| tests/functional/dynamodb/test_pagination.py | Adds functional coverage ensuring scan/query sum capacity (including index maps) and omit ConsumedCapacity when not requested. |
| tests/functional/botocore/test_paginator_config.py | Updates paginator config linter keys to allow aggregate_numeric_keys. |
| .changes/next-release/bugfix-dynamodb-49868.json | Documents the bugfix in the changelog. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
If a response member's type differed across pages (e.g. a number where an earlier page had a string, dict, or None), _deep_add_numeric could raise mid-pagination. Guard the recurse/add paths by checking the accumulator's existing type: on mismatch, preserve the first page's value (matching the documented behavior). Also deep-copy dict leaves introduced by later pages so the aggregate never aliases a source response. Does not affect DynamoDB ConsumedCapacity (its shape is stable across pages), but makes the general aggregate_numeric_keys directive robust.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Rather than overriding the (upstream-synced) non_aggregate_keys list in the overlay, treat a member declared in aggregate_numeric_keys as taking precedence: it is filtered out of non_aggregate handling in the Paginator. This keeps the overlay purely additive, so the upstream non_aggregate_keys list can keep receiving unrelated additions without being zeroed out. The paginator config linter is updated to mirror this precedence (a member aggregated across pages is only accounted for once), leaving the existing output-member validation intact.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
- _is_summable_number now uses numbers.Number (not just int/float) so decimal.Decimal values aggregate; still excludes bool. (numbers.Real is intentionally NOT used: Decimal registers as Number but not Real.) - Deep-copy list leaves (not just dict) when first inserted so the aggregate never aliases a source page. - Config linter rejects nested/dotted aggregate_numeric_keys entries, which would silently never aggregate at runtime. - Add unit tests for scalar/Decimal/boolean aggregate members and list-leaf deep-copy.
Use the same numeric types as the existing result_key aggregation (int, float) in _is_summable_number rather than numbers.Number. This keeps the two aggregation paths consistent and avoids the float+Decimal addition edge case (so no _safe_numeric_add guard is needed). Strings and booleans are still excluded, which our deep-sum semantics require (preserve TableName, never sum booleans). ConsumedCapacity.CapacityUnits is a double (float), so dropping Decimal handling has no practical effect.
Previously the directive summed every numeric leaf under a member, which would auto-aggregate any future numeric field DynamoDB might add under ConsumedCapacity that should not be summed. Change the directive to a map of member -> list of summable leaf field-names, so only named leaves (CapacityUnits, ReadCapacityUnits, WriteCapacityUnits) are totaled. Dynamic-key maps (GlobalSecondaryIndexes/LocalSecondaryIndexes) still work because matching is by leaf name during the recursive walk; any non-allowlisted numeric leaf is preserved from the first page.
The leaf-name allowlist matched a leaf by name anywhere in the subtree, so a
future field like ConsumedCapacity.AemousCapacity.CapacityUnits would be summed
even though it should not be. Change the directive to a list of explicit dotted
paths to the exact numeric leaves, with '*' matching only a dynamic key level
(e.g. ConsumedCapacity.GlobalSecondaryIndexes.*.CapacityUnits). Only the leaf at
a fully-configured path is summed; any other field (including a same-named leaf
on an unconfigured path) is preserved from the first page.
- paginate.py: _add_numeric_path walks an explicit path ('*' = any key at that
level); config parsed as member -> list of segment-tuples.
- dynamodb overlay: enumerate the exact ConsumedCapacity paths.
- linter: validate paths ('*' only interior, member is top-level).
- tests: _add_numeric_path unit tests incl. wildcard + unconfigured-sibling
safety; e2e test that a new nested same-named leaf is not summed.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
- PageIterator aggregate_numeric_keys defaults to None, normalized to {} (it is
a map used with .items(); a tuple default would AttributeError if a caller
omits it).
- _add_numeric_path returns early on empty segments (no IndexError if reused
with an empty path).
- Config linter rejects non-string entries and empty path segments (leading/
trailing or doubled '.').
| "merge": { | ||
| "pagination": { | ||
| "Query": { | ||
| "aggregate_numeric_keys": [ |
There was a problem hiding this comment.
the reason for making this new field is because i think the non_aggregate_keys field in the regular paginators.json is somehow generated. i did not touch that since it may be overwritten by a new model change.
the other reason is since we need engine changes anyway to support the GSI/LSI summation, a new key seemed more safe.
the only "weird" part of this implementation is that we just need to know that aggregate_numeric_keys takes precedence over non_aggregate_keys now
Summary
aws dynamodb scan/aws dynamodb querywith auto-pagination previously reported only the final page'sConsumedCapacity, undercounting the true consumed capacity of the whole operation (CLI-4199). The value led customers to believe their commands cost far less than they actually did.Before (a
Scanthat pages internally):Root cause
ConsumedCapacitywas anon_aggregate_key, which records a member's value from a single page and never sums it.Count/ScannedCountaggregate because they are numericresult_keys thatbuild_full_resultsums across pages.ConsumedCapacitycan't simply become aresult_key: it's a mixed dict (numericCapacityUnits, stringTableName, andGlobalSecondaryIndexes/LocalSecondaryIndexesmaps keyed by user-defined index names that no static config path can enumerate).A prior botocore attempt (boto/botocore#3055) took a config-only approach — making
ConsumedCapacity.CapacityUnitsaresult_key— which works for the statically-addressable scalar/Table.*leaves but cannot reach the index maps (dynamic keys). It was withdrawn as a draft for that reason. This change adds the engine capability that handles the dynamic maps.How the engine change works
A new opt-in paginator directive,
aggregate_numeric_keys, is a list of explicit dotted paths to the numeric leaves that should be summed across pages. A*segment matches every key at that level (for maps keyed by runtime-defined names); every other segment is matched literally:Paginatorparses each path intomember -> list of segment tuples(segments after the top-level member) and passes it to thePageIterator.build_full_resultkeeps a runningaggregate_numeric_totals. On the first page a member appears, it seeds the whole member (deep-copy). On subsequent pages it adds only the configured leaf paths via_add_numeric_path, so any field NOT on a configured path keeps its first-page value._add_numeric_pathwalks an explicit path. A*iterates every key present at that level (handlingGlobalSecondaryIndexes/LocalSecondaryIndexes, whose keys are runtime index names); a dynamic key first seen on a later page is seeded by deep-copy. Only the exact leaf at the full path is summed. Cross-page type mismatches keep the first value, and booleans are never summed._is_summable_numbermatches the same numeric types as the existingresult_keyaggregation —(int, float), excludingbool.aggregate_numeric_keyssupersedes anynon_aggregate_keysentry for the same member (filtered out of non-aggregate handling), so the overlay stays purely additive and the upstreamnon_aggregate_keyslist can keep receiving unrelated additions.Why explicit full paths (strict allowlist)
An earlier iteration allowlisted leaf names ("sum any
CapacityUnits"), but that would incorrectly sum a new field the service might add, e.g.ConsumedCapacity.AemousCapacity.CapacityUnits, just because its leaf name matched. Full paths make the allowlist strict: only the exact configured leaves are totaled. A same-named leaf on an unconfigured path — or any other new numeric field — is preserved from the first page, never auto-aggregated. The*wildcard is scoped to just the dynamic index-name level so index maps still aggregate.The directive is opt-in per paginator config, so no other service's pagination behavior changes, and the pre-existing
result_keyaggregation path is untouched.Behavior notes
aws ddbcommands, which build a full aggregated result.--output textstreams pages and can't produce a running total; it no longer emits the (never-correct, first-page-only)ConsumedCapacityduring pagination.ConsumedCapacityis simply absent (the previous config could emit"ConsumedCapacity": null).Testing
_add_numeric_path(static path, nested path,*dynamic key incl. new index on a later page, unconfigured same-named sibling not summed, missing path no-op, type mismatch, boolean) and end-to-endaggregate_numeric_keysaggregation incl. theAemousCapacitystrictness case (tests/unit/botocore/test_paginate.py).scan/querysumming capacity incl. secondary-index maps and the not-requested case (tests/functional/dynamodb/test_pagination.py).*only interior; top-level member).ConsumedCapacityequals the sum of the individual pages, and the per-index map aggregates under its runtime index name.