Skip to content
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

[fix] Restored method as deprecated in AbstractMetadataStore #21950

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

merlimat
Copy link
Contributor

Motivation

A change in #21068 has renamed a protected methods in AbstractMetadataStore. This was done in 3.1 and 3.2 as well as master branch.

The problem with this change is that external metadata provider plugins cannot easily target 3.0/3.1 release because of the method name change. We shouldn't be renaming methods part of the plugin APIs. Instead we can mark them as deprecated and maintain compatibility.

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

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

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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:

@merlimat merlimat added type/bug The PR fixed a bug or issue reported a bug release/3.1.3 release/3.2.1 labels Jan 23, 2024
@merlimat merlimat added this to the 3.3.0 milestone Jan 23, 2024
@merlimat merlimat self-assigned this Jan 23, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 23, 2024
@codelipenghui
Copy link
Contributor

codelipenghui commented Jan 23, 2024

@merlimat

We should also add annotations

@InterfaceAudience.Public
@InterfaceStability.Stable

Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

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

Please add a comment.

@Technoboy- Technoboy- requested a review from nodece January 24, 2024 03:10
@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (22838ea) 36.46% compared to head (531d8ff) 73.61%.
Report is 16 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #21950       +/-   ##
=============================================
+ Coverage     36.46%   73.61%   +37.15%     
- Complexity    12390    32439    +20049     
=============================================
  Files          1725     1861      +136     
  Lines        131701   138676     +6975     
  Branches      14401    15182      +781     
=============================================
+ Hits          48027   102093    +54066     
+ Misses        77254    28700    -48554     
- Partials       6420     7883     +1463     
Flag Coverage Δ
inttests 24.08% <0.00%> (-0.03%) ⬇️
systests 23.65% <0.00%> (-0.02%) ⬇️
unittests 72.90% <0.00%> (+40.85%) ⬆️

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

Files Coverage Δ
...he/pulsar/metadata/impl/AbstractMetadataStore.java 84.55% <0.00%> (+30.48%) ⬆️

... and 1442 files with indirect coverage changes

@nodece nodece merged commit 4198ed2 into apache:master Jan 24, 2024
47 checks passed
@Technoboy- Technoboy- modified the milestones: 3.3.0, 3.2.0 Jan 27, 2024
Technoboy- added a commit that referenced this pull request Jan 27, 2024
Co-authored-by: Jiwe Guo <technoboy@apache.org>
@Technoboy-
Copy link
Contributor

Since #21068 does't cherry-pick to branch-3.1, so we remove the label release/3.1.3 fro this patch

lhotari pushed a commit that referenced this pull request Nov 28, 2024
Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 4198ed2)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 28, 2024
…21950)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 4198ed2)
(cherry picked from commit a01f26d)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 28, 2024
…21950)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 4198ed2)
(cherry picked from commit a01f26d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.8 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants