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

[improve][ws] Support subscribing multi/pattern topic for Websocket #21379

Merged

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Oct 17, 2023

PIP-307

Motivation

Support subscribing multi/pattern topic for WS.
The new sub-path could be :

/ws/v3/consumer/subscription?topicsPattern="a.*" /ws/v3/consumer/subscription?topics="a,b,c"

Documentation

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

@Technoboy- Technoboy- self-assigned this Oct 17, 2023
@Technoboy- Technoboy- added this to the 3.2.0 milestone Oct 17, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 17, 2023
Copy link
Member

@mattisonchao mattisonchao left a comment

Choose a reason for hiding this comment

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

Overal LGTM.

@mattisonchao
Copy link
Member

Please help fix the checkstyle issue.

@Technoboy- Technoboy- force-pushed the support-multi-topic-consume-for-ws branch from 59e07f7 to 6caf1ec Compare October 30, 2023 02:38
@codecov-commenter
Copy link

Codecov Report

Merging #21379 (6caf1ec) into master (bd86e4e) will increase coverage by 38.71%.
The diff coverage is 58.82%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #21379       +/-   ##
=============================================
+ Coverage     34.54%   73.25%   +38.71%     
- Complexity    12084    32609    +20525     
=============================================
  Files          1713     1892      +179     
  Lines        130782   140444     +9662     
  Branches      14250    15436     +1186     
=============================================
+ Hits          45173   102878    +57705     
+ Misses        79577    29475    -50102     
- Partials       6032     8091     +2059     
Flag Coverage Δ
inttests 24.14% <23.52%> (?)
systests 24.72% <13.23%> (-0.02%) ⬇️
unittests 72.54% <55.88%> (+40.67%) ⬆️

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

Files Coverage Δ
...n/java/org/apache/pulsar/broker/PulsarService.java 83.07% <100.00%> (+15.67%) ⬆️
...pache/pulsar/proxy/server/ProxyServiceStarter.java 66.44% <100.00%> (+34.44%) ⬆️
...che/pulsar/websocket/AbstractWebSocketHandler.java 57.55% <100.00%> (+57.55%) ⬆️
...a/org/apache/pulsar/websocket/ConsumerHandler.java 63.09% <100.00%> (+63.09%) ⬆️
...sar/websocket/service/WebSocketServiceStarter.java 74.46% <100.00%> (+74.46%) ⬆️
.../websocket/WebSocketMultiTopicConsumerServlet.java 88.88% <88.88%> (ø)
...he/pulsar/websocket/MultiTopicConsumerHandler.java 38.63% <38.63%> (ø)

... and 1468 files with indirect coverage changes

@Technoboy- Technoboy- merged commit eebd821 into apache:master Nov 6, 2023
@Technoboy- Technoboy- deleted the support-multi-topic-consume-for-ws branch November 11, 2023 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/websocket doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants