-
Notifications
You must be signed in to change notification settings - Fork 2.1k
CFP Consistent TS Format #46784
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
CFP Consistent TS Format #46784
Conversation
|
/azp run java - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull Request Overview
This PR standardizes timestamp format in Change Feed Processor (CFP) lease objects by converting from ZonedDateTime.toString() to Instant.toString() format. The change removes inconsistent "[UTC]" suffixes that were appearing in some timestamps.
- Updated timestamp generation in ServiceItemLease and ServiceItemLeaseV1 classes to use Instant format
- Added comprehensive test to verify timestamp consistency across CFP lifecycle
- Updated changelog to document the formatting change
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ServiceItemLease.java | Changed timestamp generation from ZonedDateTime.toString() to toInstant().toString() |
| ServiceItemLeaseV1.java | Applied same timestamp format change for consistency |
| CHANGELOG.md | Added entry documenting the timestamp format change |
| IncrementalChangeFeedProcessorTest.java | Added test to verify timestamps don't contain "[UTC]" suffix |
.../test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java
Outdated
Show resolved
Hide resolved
.../test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java
Outdated
Show resolved
Hide resolved
.../test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java
Show resolved
Hide resolved
.../test/java/com/azure/cosmos/rx/changefeed/epkversion/IncrementalChangeFeedProcessorTest.java
Show resolved
Hide resolved
…to tvaron3/changefeedprocessorTS # Conflicts: # sdk/cosmos/azure-cosmos/CHANGELOG.md
.../src/main/java/com/azure/cosmos/implementation/changefeed/epkversion/ServiceItemLeaseV1.java
Show resolved
Hide resolved
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.
This is a breakign change - would need either to be gated on a new Lease version - I am not convinced that it is worth it
…to tvaron3/changefeedprocessorTS # Conflicts: # sdk/cosmos/azure-cosmos/CHANGELOG.md
Discussed offline - since the lease document in a flow of Would have used the same format as now in T0 in T1 and later already this is not a breakign change but just a bug fix. |
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
Description
Timestamp format was inconsistent in lease throughout life cycle of change feed processor. Initial timestamp when a lease is created has an appended [utc] because a different data object is used at lease creation than during updates. Once change feed processor starts processing the lease, the lease will get updated and the format of the timestamp is changed.
Current Flow
New flow