Skip to content

[improve][client] Deprecate MessageIdUtils.getOffset and MessageIdUtils.getMessageId#22747

Merged
RobertIndie merged 1 commit intoapache:masterfrom
RobertIndie:deprecate-get-offset
May 21, 2024
Merged

[improve][client] Deprecate MessageIdUtils.getOffset and MessageIdUtils.getMessageId#22747
RobertIndie merged 1 commit intoapache:masterfrom
RobertIndie:deprecate-get-offset

Conversation

@RobertIndie
Copy link
Copy Markdown
Member

@RobertIndie RobertIndie commented May 20, 2024

Motivation

After discussing here, the pulsar client shouldn't expose the offset term to users.

Modifications

  • Deprecate MessageIdUtils.getOffset and MessageIdUtils.getMessageId
  • For connectors, use FunctionCommon.getOffset and FunctionCommon.getMessageId

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@RobertIndie RobertIndie self-assigned this May 20, 2024
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label May 20, 2024
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 21, 2024

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.17%. Comparing base (bbc6224) to head (3825d6b).
⚠️ Report is 1318 commits behind head on master.

Files with missing lines Patch % Lines
...r/io/kafka/connect/PulsarKafkaSinkTaskContext.java 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22747      +/-   ##
============================================
- Coverage     73.57%   73.17%   -0.40%     
+ Complexity    32624    32560      -64     
============================================
  Files          1877     1889      +12     
  Lines        139502   141419    +1917     
  Branches      15299    15518     +219     
============================================
+ Hits         102638   103486     +848     
- Misses        28908    29936    +1028     
- Partials       7956     7997      +41     
Flag Coverage Δ
inttests 27.54% <ø> (+2.96%) ⬆️
systests 24.60% <ø> (+0.27%) ⬆️
unittests 72.18% <0.00%> (-0.67%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../org/apache/pulsar/client/util/MessageIdUtils.java 0.00% <ø> (-88.89%) ⬇️
...r/io/kafka/connect/PulsarKafkaSinkTaskContext.java 0.00% <0.00%> (-64.04%) ⬇️

... and 352 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RobertIndie RobertIndie merged commit 878a412 into apache:master May 21, 2024
@lhotari lhotari added this to the 4.0.0 milestone Oct 14, 2024
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
…Utils.getMessageId` (apache#22747)

### Motivation

After discussing [here](apache#22698 (comment)),  the pulsar client shouldn't expose the `offset` term to users.

### Modifications

- Deprecate `MessageIdUtils.getOffset` and `MessageIdUtils.getMessageId`
- For connectors, use `FunctionCommon.getOffset` and `FunctionCommon.getMessageId`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants