-
Notifications
You must be signed in to change notification settings - Fork 50
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
History type support #176
History type support #176
Conversation
- Use client_tag_hash as ID for history type - No tag items needed for history type - No version checking for history type when update records
f1cf8aa
to
d4e8a43
Compare
d4e8a43
to
57be8f4
Compare
schema/dynamodb/table.json
Outdated
@@ -12,7 +12,7 @@ | |||
], | |||
"GlobalSecondaryIndexes": [ | |||
{ | |||
"IndexName": "ClientIDDataTypeMtimeIndex", | |||
"IndexName": "ClientIDDataTypeMtimeIndexV2", |
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.
a db change is needed for this to work:
- enabling ttl for the
ExpirationTime
attribute
f9c2404
to
a1ec741
Compare
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.
Overall looks good, just some minor comments for readability.
Also please make sure to have a green CI test.
a1ec741
to
862e79d
Compare
log.Error().Err(err).Msg("Insert history sync entity failed") | ||
rspType := sync_pb.CommitResponse_TRANSIENT_ERROR | ||
entryRsp.ResponseType = &rspType | ||
entryRsp.ErrorMessage = aws.String(fmt.Sprintf("Insert history sync entity failed: %v", err.Error())) | ||
continue |
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.
nit: Maybe Error checking if client_unique_tag exists
rather than Insert history sync entity failed
is more clear?
Adds support for the new Chromium history type.
History sync entities will use the
ClientTagHash
as a primary key, instead of theIdString
. Differing version numbers between old and new items will be ignored when updating.