feat: bulksubscribe http

[sadath-12]

Fixes #423 for http bulksubscribe

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 75.36232% with 34 lines in your changes are missing coverage. Please review.

Project coverage is 58.63%. Comparing base (27248ba) to head (300e5ac). Report is 11 commits behind head on main.

Files	Patch %	Lines
service/http/topic.go	66.66%	21 Missing and 7 partials :warning:
service/internal/topicregistrar.go	86.66%	2 Missing and 2 partials :warning:
service/internal/topicsubscription.go	75.00%	1 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #478 +/- ## ========================================== + Coverage 58.04% 58.63% +0.58% ========================================== Files 55 55 Lines 3568 3701 +133 ========================================== + Hits 2071 2170 +99 - Misses 1375 1399 +24 - Partials 122 132 +10 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[daixiang0]

Codes look good, please add unit tests.

[sadath-12]

Added it 🙂

[sadath-12]

Thank you for the review @mikeee . Added them

[daixiang0]

Please fix conflicts.

[sadath-12]

Done @daixiang0

[sadath-12]

thanks , I din't notice that @mikeee fixed !

[daixiang0]

Please make CI happy.

[sadath-12]

Yup @daixiang0 it seems to be chilling now . 😅 Thanks

[sadath-12]

I've missed this over the past few reviews but Redis is not suitable for testing bulk pub/sub. Would it be wise to migrate to another broker (for example kafka or I believe ASB) so that we can validate this example?

I'd like to see a validated run that doesn't fallback to singular pub/sub before I review again?

can we keep the migration in separate pr ?

[mikeee]

I've missed this over the past few reviews but Redis is not suitable for testing bulk pub/sub. Would it be wise to migrate to another broker (for example kafka or I believe ASB) so that we can validate this example? I'd like to see a validated run that doesn't fallback to singular pub/sub before I review again?

can we keep the migration in separate pr ?

I have to answer that with another question - has the bulk pub/sub been validated elsewhere? All I'm seeing is fallbacks.

[sadath-12]

I've missed this over the past few reviews but Redis is not suitable for testing bulk pub/sub. Would it be wise to migrate to another broker (for example kafka or I believe ASB) so that we can validate this example? I'd like to see a validated run that doesn't fallback to singular pub/sub before I review again?

can we keep the migration in separate pr ?

I have to answer that with another question - has the bulk pub/sub been validated elsewhere? All I'm seeing is fallbacks.

sure no issues lets test it for others as well . Before I jump wanted to confirm what you mean by fallback here ? because when I see the demo of bulksubscribe in the dapr docs and how js-sdk has implemented bulksubscribe . I have made sure similar behaviour is achieved here . would you like to explain what do you expect clearly ? Happy to implement whatever works best for the project 😊 If possible maybe we could drive the talk on the discord as well since some discussion is required on this

[mikeee]

I've missed this over the past few reviews but Redis is not suitable for testing bulk pub/sub. Would it be wise to migrate to another broker (for example kafka or I believe ASB) so that we can validate this example? I'd like to see a validated run that doesn't fallback to singular pub/sub before I review again?

can we keep the migration in separate pr ?

I have to answer that with another question - has the bulk pub/sub been validated elsewhere? All I'm seeing is fallbacks.

sure no issues lets test it for others as well . Before I jump wanted to confirm what you mean by fallback here ? because when I see the demo of bulksubscribe in the dapr docs and how js-sdk has implemented bulksubscribe . I have made sure similar behaviour is achieved here . would you like to explain what do you expect clearly ? Happy to implement whatever works best for the project 😊 If possible maybe we could drive the talk on the discord as well since some discussion is required on this

The issue is that since the broker used does not implement bulk pub/sub methods, we are effectively dropping down to single pub/sub. Looking at the validation run it is highlighted in the logs, likewise in the js-sdk I note this is an issue too as it uses rabbitmq but this is not validated so you'd run into the same result if run in debug mode.

[sadath-12]

I've missed this over the past few reviews but Redis is not suitable for testing bulk pub/sub. Would it be wise to migrate to another broker (for example kafka or I believe ASB) so that we can validate this example? I'd like to see a validated run that doesn't fallback to singular pub/sub before I review again?

can we keep the migration in separate pr ?

I have to answer that with another question - has the bulk pub/sub been validated elsewhere? All I'm seeing is fallbacks.

sure no issues lets test it for others as well . Before I jump wanted to confirm what you mean by fallback here ? because when I see the demo of bulksubscribe in the dapr docs and how js-sdk has implemented bulksubscribe . I have made sure similar behaviour is achieved here . would you like to explain what do you expect clearly ? Happy to implement whatever works best for the project 😊 If possible maybe we could drive the talk on the discord as well since some discussion is required on this

The issue is that since the broker used does not implement bulk pub/sub methods, we are effectively dropping down to single pub/sub. Looking at the validation run it is highlighted in the logs, likewise in the js-sdk I note this is an issue too as it uses rabbitmq but this is not validated so you'd run into the same result if run in debug mode.

Ya the dapr itself sends back the response one by one . so with all those brokers we would get the same result right ? we run the callback for each entry and send response to dapr

[sadath-12]

@mikeee what you want to say about this approach are we going up with this pr after those things resolved ?

[mikeee]

@mikeee what you want to say about this approach are we going up with this pr after those things resolved ?

I think we should definitely try to get this merged even as a fallback validation, as long as there is validation locally just to make sure it's working then that should be fine 👍

[Eileen-Yu]

wondering if there's any progress on this PR?

I'm happy to help finish this one. Would you please resolve those conflicts if you'd like me to keep your commits? @sadath-12

[sadath-12]

@Eileen-Yu

[sicoyle]

@mikeee any other feedback on this PR?