Skip to content

[Issue 14013][broker] make triggerOffload method async#14027

Merged
codelipenghui merged 2 commits into
apache:masterfrom
HQebupt:issue-14013-triggerOffload
Feb 8, 2022
Merged

[Issue 14013][broker] make triggerOffload method async#14027
codelipenghui merged 2 commits into
apache:masterfrom
HQebupt:issue-14013-triggerOffload

Conversation

@HQebupt

@HQebupt HQebupt commented Jan 29, 2022

Copy link
Copy Markdown
Contributor

Master Issue: #14013

Motivation

See #14013

Verifying this change

Make sure that the change passes the CI checks.

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jan 29, 2022
@HQebupt HQebupt changed the title [Issue 13854][broker] make triggerOffload method async [Issue 14013][broker] make triggerOffload method async Jan 29, 2022
@HQebupt

HQebupt commented Jan 29, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@HQebupt
HQebupt force-pushed the issue-14013-triggerOffload branch from 914a691 to 061c9dc Compare January 29, 2022 04:48
@HQebupt
HQebupt force-pushed the issue-14013-triggerOffload branch from 803d9e2 to 003711a Compare January 29, 2022 06:49
@HQebupt

HQebupt commented Jan 29, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 29, 2022

@mattisonchao mattisonchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM +1.
but I have a suggestion:

  • Remove asyncResponse from the internal method, and unified exception and result handling are performed at the controller layer, which makes the internal method purer.

@HQebupt

HQebupt commented Jan 30, 2022

Copy link
Copy Markdown
Contributor Author

@Jason918 PTAL, thanks

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.

4 participants