Skip to content

Conversation

j7nw4r
Copy link
Member

@j7nw4r j7nw4r commented Oct 8, 2025

This PR addresses: #3073

Properly retrieve the offset and sequence numbers to properly store checkpoints.

@Copilot Copilot AI review requested due to automatic review settings October 8, 2025 02:11
Copy link
Contributor

@Copilot Copilot AI left a 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 fixes the improper extraction of offset and sequence numbers for Event Hub checkpoints. The existing code was manually parsing AMQP message annotations to retrieve these values, which was error-prone and redundant. The fix leverages the existing accessor methods on ReceivedEventData to directly obtain the offset and sequence number values.

  • Removes manual AMQP annotation parsing logic for offset and sequence numbers
  • Uses direct accessor methods event_data.offset() and event_data.sequence_number() instead
  • Simplifies the checkpoint update flow by removing unnecessary error handling for annotation parsing

@j7nw4r j7nw4r force-pushed the j7nw4r/partitionclient-fix branch from baa20de to c22a587 Compare October 8, 2025 02:31
@j7nw4r
Copy link
Member Author

j7nw4r commented Oct 8, 2025

@LarryOsterman Please review. I went back to a structure similar to the original. I updated the annotation labels used to use the same labels as the .offset() and .sequence_number() functions. I also cleaned up the logic to make things more readable (imo).

@j7nw4r
Copy link
Member Author

j7nw4r commented Oct 8, 2025

@microsoft-github-policy-service agree company="Microsoft"

@LarryOsterman
Copy link
Member

/cla help

@LarryOsterman
Copy link
Member

@microsoft-github-policy-service rerun

1 similar comment
@LarryOsterman
Copy link
Member

@microsoft-github-policy-service rerun

@heaths
Copy link
Member

heaths commented Oct 8, 2025

/check-enforcer evaluate

Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

With this in, can we get a release out? I have #3143 in a draft state waiting on this, more or less.

@j7nw4r j7nw4r merged commit adff670 into Azure:main Oct 8, 2025
17 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.

3 participants