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

Minibatch dynamodb client #652

Merged
merged 8 commits into from
Jul 26, 2024
Merged

Minibatch dynamodb client #652

merged 8 commits into from
Jul 26, 2024

Conversation

pschork
Copy link
Contributor

@pschork pschork commented Jul 20, 2024

Why are these changes needed?

Minibatch dynamodb client
Schema uses single table design

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.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@pschork pschork requested a review from ian-shim July 20, 2024 23:45
@pschork pschork force-pushed the pschork/minibatch_client branch 2 times, most recently from cf40593 to 45f9a09 Compare July 25, 2024 10:07
@pschork pschork marked this pull request as ready for review July 25, 2024 10:07
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.

lgtm! do you want #664 merged and rebase this one on master?

KeyType: types.KeyTypeHash,
},
{
AttributeName: aws.String("CreatedAt"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this index records that don't have CreatedAt attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. All items indexed by a GSI must have values for the attributes that are part of the GSI's key schema. If we had used CreatedAt (as opposed to RequestedAt) for DispersalRequest, DispersalRequests still would not have been included in the GSI because DispersalRequest does not define BatchStatus

@pschork
Copy link
Contributor Author

pschork commented Jul 26, 2024

lgtm! do you want #664 merged and rebase this one on master?

pls do. I just approved

@pschork
Copy link
Contributor Author

pschork commented Jul 26, 2024

lgtm! do you want #664 merged and rebase this one on master?

pls do. I just approved

Correct PR #665

pschork added 8 commits July 25, 2024 22:26
Replace DispersalRequest.OperatorID field with raw byte array def so that dynamo can use unmarshalMap
Convert PK to BatchID
Adds custom minibatch struct conversion for batch, minibatch, dispersalRequest, dispersalResponse
Add remaining minibatch store functions
Remove blobMetadataIndex
Add unique constraint on (batchID,BATCH#batchID)
@pschork pschork force-pushed the pschork/minibatch_client branch from b6deafb to 4551c2a Compare July 26, 2024 05:27
@pschork pschork merged commit 414f0ab into master Jul 26, 2024
9 checks passed
pschork added a commit that referenced this pull request Jul 28, 2024
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