-
Notifications
You must be signed in to change notification settings - Fork 497
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
Change feed: Adds id and pk to ChangeFeedMetadata for delete operations #4922
base: master
Are you sure you want to change the base?
Conversation
/// The id of the previous item version. Used for delete operations only. | ||
/// </summary> | ||
[JsonProperty(PropertyName = ChangeFeedMetadataFields.DeletedItemId, NullValueHandling = NullValueHandling.Ignore)] | ||
public string DeletedItemId { get; internal set; } |
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.
I understand that the backend only returns Deleted Item information for now but in the future it will also return update information and so on. Do you think it would be better to name this field as ItemId instead of DeletedItemId? We can update our documentation to reflect this fact.
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.
Id and partition key in the metadata section will always only be for delete. I added the DeletedItem prefix to disambiguate that, otherwise customers may expect it to be populated for every operation.
You may be thinking of Previous
which will eventually be populated for both deletes and updates.
/// The partition key of the previous item version. Dictionary Key is the partition key property name and Dictionary Value is the partition key property value. Used for delete operations only. | ||
/// </summary> | ||
[JsonProperty(PropertyName = ChangeFeedMetadataFields.DeletedItemPartitionKey, NullValueHandling = NullValueHandling.Ignore)] | ||
public Dictionary<string, string> DeletedItemPartitionKey { get; internal set; } |
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.
I would recommend checking with Kiran/Fabien on how the Hierarchical keys are represented with in our codebase so that we are consistent but otherwise I would mark it as Dictionary<string, object> as the value could of any type.
Pull Request Template
Description
Adds id and partition key to ChangeFeedMetadata. This is populated for delete operations using all versions and deletes change feed mode.
Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #IssueNumber