Skip to content
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

[payments] numeric incremental updates, query with ordering and lim #788

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

hopeyen
Copy link
Contributor

@hopeyen hopeyen commented Oct 7, 2024

Why are these changes needed?

  • New DynamoDB client functions to handle metering offchain payment information

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@hopeyen hopeyen force-pushed the hope/dynamodb-updates branch 2 times, most recently from 03900ad to 8cc0e7b Compare October 7, 2024 22:49
@hopeyen hopeyen changed the title feat: numeric incremental updates, query with ordering and lim numeric incremental updates, query with ordering and lim Oct 8, 2024
@hopeyen hopeyen changed the title numeric incremental updates, query with ordering and lim [payments] numeric incremental updates, query with ordering and lim Oct 8, 2024
@hopeyen hopeyen force-pushed the hope/dynamodb-updates branch 4 times, most recently from 131ee44 to 1692ab3 Compare October 8, 2024 17:53
Copy link
Contributor

@ian-shim ian-shim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add tests for the new methods in client_test.go

@@ -156,6 +157,35 @@ func (c *Client) UpdateItem(ctx context.Context, tableName string, key Key, item
return resp.Attributes, err
}

func (c *Client) IncrementBy(ctx context.Context, tableName string, key Key, itemKey string, itemValue uint64) (Item, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: attr string, value uint64?
(itemValue sounds like it will be assigned this new value)

@hopeyen hopeyen force-pushed the hope/dynamodb-updates branch 2 times, most recently from feb5543 to a8a6ca9 Compare October 8, 2024 23:55
Copy link
Contributor

@ian-shim ian-shim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment, lgtm otherwise

@@ -191,6 +222,23 @@ func (c *Client) QueryIndex(ctx context.Context, tableName string, indexName str
return response.Items, nil
}

// QueryIndexOrderWithLimit returns all items in the index that match the given key
func (c *Client) QueryIndexOrderWithLimit(ctx context.Context, tableName string, indexName string, keyCondition string, expAttributeValues ExpresseionValues, forward bool, limit int32) ([]Item, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for this method as well?

@hopeyen hopeyen force-pushed the hope/dynamodb-updates branch from 1dec83c to af1d9fe Compare October 9, 2024 19:37
@hopeyen hopeyen force-pushed the hope/dynamodb-updates branch from af1d9fe to a6810c2 Compare October 9, 2024 19:40
@hopeyen hopeyen merged commit 9910b72 into master Oct 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants