FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[feature][broker] PIP-204: Extensions for broker interceptor by aloyszhang · Pull Request #17269 · apache/pulsar · GitHub

/ pulsar Public

[feature][broker] PIP-204: Extensions for broker interceptor - #17269

Merged
codelipenghui merged 1 commit into
apache:masterfrom
aloyszhang:interceptor
Sep 15, 2022
ConversationCommits (1)ChecksFiles changed
Merged

[feature][broker] PIP-204: Extensions for broker interceptor#17269
codelipenghui merged 1 commit into
apache:masterfrom
aloyszhang:interceptor

Conversation

aloyszhang commented Aug 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #17267

Motivation

extend BrokerInterceptor, details see #17267

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: ( yes)
  • 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

  • doc-not-needed

github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Aug 25, 2022

codelipenghui commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

@aloyszhang It should be a public API change, the proposal is required so that we can make everyone on the same page that how the new API will be used by users.

aloyszhang commented Aug 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@codelipenghui Thanks, I'll send a discuss email to dev soon.

aloyszhang commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

Update PIP discuss thread : https://lists.apache.org/thread/3zgpbxffo7gzsb5mdh0sk2rq27bgw2bo
cc @codelipenghui

aloyszhang requested a review from Jason918 August 26, 2022 02:23

aloyszhang commented Sep 1, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

aloyszhang commented Sep 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The vote(https://lists.apache.org/thread/ckxqgoyjkqgbp6szn2vh9ynzbsjxm3yy) for this PIP is now passed.
PTAL @codelipenghui @Jason918 @eolivelli @Technoboy- , thanks.

aloyszhang changed the title extend BrokerInterceptor for more scenes [feature][broker]extend BrokerInterceptor for more scenes Sep 5, 2022

AnonHxy commented Sep 6, 2022

Copy link
Copy Markdown
Contributor

LGTM

wolfstudy approved these changes Sep 6, 2022
View reviewed changes

wolfstudy 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM +1

eolivelli approved these changes Sep 6, 2022
View reviewed changes

eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Lgtm

aloyszhang commented Sep 7, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

1 similar comment

aloyszhang commented Sep 8, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

Jason918 added type/feature The PR added a new feature or issue requested a new feature area/broker labels Sep 8, 2022
Jason918 changed the title [feature][broker]extend BrokerInterceptor for more scenes [feature][broker] PIP-204: Extensions for broker interceptor Sep 8, 2022
codelipenghui reviewed Sep 9, 2022
View reviewed changes

codelipenghui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think you have removed the test coverage?

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/intercept/BrokerInterceptors.java
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java Outdated
Comment on lines +329 to +331
if (brokerInterceptor != null) {
brokerInterceptor.producerClosed(this, producer, producer.getMetadata());
}

codelipenghui Sep 9, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like we don't need if check here?

aloyszhang commented Sep 9, 2022

Copy link
Copy Markdown
Contributor Author

I think you have removed the test coverage?

which part do you mean？

aloyszhang force-pushed the interceptor branch 2 times, most recently from 06a42b3 to b264a92 Compare September 9, 2022 12:58
codelipenghui merged commit c7b7146 into apache:master Sep 15, 2022
codertmy pushed a commit to codertmy/pulsar that referenced this pull request Sep 16, 2022
[feature][broker] PIP-204: Extensions for broker interceptor (apache#…
df90351 
…17269)
aloyszhang deleted the interceptor branch November 30, 2022 01:44
BewareMyPower mentioned this pull request Sep 26, 2023
Merged
4 tasks
nodece mentioned this pull request Nov 1, 2023
Merged
4 tasks
nodece added this to the 3.0 milestone Nov 10, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Jason918 Jason918 approved these changes
eolivelli eolivelli approved these changes
AnonHxy AnonHxy approved these changes
codelipenghui codelipenghui approved these changes
wolfstudy wolfstudy approved these changes
BewareMyPower Awaiting requested review from BewareMyPower
gaoran10 Awaiting requested review from gaoran10
Technoboy- Awaiting requested review from Technoboy-

Assignees

aloyszhang

Labels

area/broker doc-not-needed Your PR changes do not impact docs type/feature The PR added a new feature or issue requested a new feature

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

PIP-204 Extensions for broker interceptor

7 participants

Back | FazBrowse Home | New Git URL