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][ml] Fix unfinished callback when deleting managed ledger #21530

Merged
merged 2 commits into from
Nov 10, 2023
Merged

[fix][ml] Fix unfinished callback when deleting managed ledger #21530

merged 2 commits into from
Nov 10, 2023

Conversation

mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Nov 7, 2023

Motivation

#7131 introduced an unfinished callback.

Modifications

  • Complete the callback with an exception to let the user retry.

  • doc-not-needed

Verifying this change

  • Make sure that the change passes the CI checks.

@mattisonchao mattisonchao self-assigned this Nov 7, 2023
@mattisonchao mattisonchao added the category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost label Nov 7, 2023
Copy link

github-actions bot commented Nov 7, 2023

@mattisonchao Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 7, 2023
@mattisonchao mattisonchao added this to the 3.2.0 milestone Nov 7, 2023
@mattisonchao mattisonchao reopened this Nov 7, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 7, 2023
Copy link
Member

@coderzc coderzc left a comment

Choose a reason for hiding this comment

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

Can you add a test to cover it?

@mattisonchao
Copy link
Member Author

Yes, sure. @coderzc

Copy link
Member

@Demogorgon314 Demogorgon314 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 unit test to cover it.

@Technoboy- Technoboy- merged commit a8f22d8 into apache:master Nov 10, 2023
65 of 75 checks passed
@mattisonchao mattisonchao deleted the fix/un_finished_future branch November 10, 2023 13:13
nborisov pushed a commit to nborisov/pulsar that referenced this pull request Nov 13, 2023
nodece pushed a commit to nodece/pulsar that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants