-
Notifications
You must be signed in to change notification settings - Fork 28
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
Update reduceCapacity to support new data shape #157
Conversation
Noticed that there were a few more fields missing from the current doc: Items: Array<Map<Map>>
Count: Integer
ScannedCount: Integer
LastEvaluatedKey: Map<Map>
ConsumedCapacity: Map
TableName: String
CapacityUnits: Float
ReadCapacityUnits: Float
WriteCapacityUnits: Float
Table: map
ReadCapacityUnits: Float
WriteCapacityUnits: Float
CapacityUnits: Float
LocalSecondaryIndexes: Map<Map>
ReadCapacityUnits: Float
WriteCapacityUnits: Float
CapacityUnits: Float
GlobalSecondaryIndexes: Map<Map>
ReadCapacityUnits: Float
WriteCapacityUnits: Float
CapacityUnits: Float |
{ CapacityUnits: existing.LocalSecondaryIndexes.CapacityUnits + incoming.LocalSecondaryIndexes.CapacityUnits } : | ||
{ CapacityUnits: incoming.LocalSecondaryIndexes.CapacityUnits }; | ||
function mergeCapacityParents(dst, src, k) { | ||
if (!src[k]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we check if src or dst is null in case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src
can't be null here because we're not calling this function if it is (on L55) (TypeScript would be helpful in enforcing that, but we don't have it here), and we init dst
on L46-47.
There's a test case already that checks this code path: [reduceCapacity] parses new data format correctly
, there dst
is built up from nothing.
But you're right that this is not obvious at all without compiler's help and someone in the future might break that accidentally. The tests would show it, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, I would like to add null/undefined checking before accessing its properties to avoid unexpected exception thrown, especially this is an utils function, try to make it more common and robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
When using batch operations the stats are not returned correctly.
For example, this request:
causes AWS to return the following data structure:
which is then transformed by
reduceCapacity
insidesendAll
to this:This PR fixes that problem.