Skip to content

Implement Bulk Publish functionality#1001

Merged
halspang merged 2 commits into
dapr:masterfrom
yash-nisar:bulk_publish
Jan 26, 2023
Merged

Implement Bulk Publish functionality#1001
halspang merged 2 commits into
dapr:masterfrom
yash-nisar:bulk_publish

Conversation

@yash-nisar

Copy link
Copy Markdown
Contributor

Signed-off-by: Yash Nisar [email protected]

Description

Implement Bulk Publish functionality

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #958

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@yash-nisar
yash-nisar requested review from a team as code owners December 23, 2022 04:18
@codecov

codecov Bot commented Dec 23, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1001 (6274dd5) into master (76d4b68) will increase coverage by 0.22%.
The diff coverage is 92.98%.

@@            Coverage Diff             @@
##           master    #1001      +/-   ##
==========================================
+ Coverage   69.92%   70.15%   +0.22%     
==========================================
  Files         157      160       +3     
  Lines        5227     5284      +57     
  Branches      562      567       +5     
==========================================
+ Hits         3655     3707      +52     
- Misses       1439     1443       +4     
- Partials      133      134       +1     
Flag Coverage Δ
net6 70.07% <92.98%> (+0.24%) ⬆️
net7 70.07% <92.98%> (+0.24%) ⬆️
netcoreapp3.1 70.11% <92.98%> (+0.23%) ⬆️

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

Impacted Files Coverage Δ
src/Dapr.Client/DaprClient.cs 92.59% <ø> (ø)
src/Dapr.Client/BulkPublishEntry.cs 70.00% <70.00%> (ø)
src/Dapr.Client/DaprClientGrpc.cs 87.27% <97.29%> (+0.39%) ⬆️
src/Dapr.Client/BulkPublishResponse.cs 100.00% <100.00%> (ø)
src/Dapr.Client/BulkPublishResponseFailedEntry.cs 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shubham1172

Copy link
Copy Markdown
Member

Hello @yash-nisar, did you get a chance to look into dapr/sig-sdk-spec#22? It's still something I am figuring out, but with implementations across SDKs we can make both the SIG specification and SDK PRs consistent.

@yash-nisar

yash-nisar commented Jan 2, 2023

Copy link
Copy Markdown
Contributor Author

Hey @shubham1172 , not yet. Will look at it and make the necessary modifications. However, I've mostly tried to be consistent with dapr/java-sdk#789. Thanks for bringing this up !

Comment thread examples/Client/PublishSubscribe/BulkPublishEventExample/Program.cs Outdated
Comment thread src/Dapr.Client/DaprClient.cs Outdated
Comment thread src/Dapr.Client/DaprClient.cs Outdated
Comment thread src/Dapr.Client/DaprClient.cs Outdated
Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated
Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated
Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated

@halspang halspang 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.

Code looks fine, I think you just need to update the example README to state which subscriber needs to be running.

Comment thread examples/Client/PublishSubscribe/BulkPublishEventExample/README.md Outdated
@halspang

Copy link
Copy Markdown
Contributor

Oh, and please fix the merge conflicts.

@yash-nisar
yash-nisar force-pushed the bulk_publish branch 2 times, most recently from 61a4faf to bf9f966 Compare January 25, 2023 03:19
@yash-nisar
yash-nisar requested a review from halspang January 25, 2023 03:25
Comment thread examples/Client/PublishSubscribe/BulkPublishEventExample/Example.cs Outdated
Comment thread examples/Client/PublishSubscribe/BulkPublishEventExample/README.md Outdated
Comment thread src/Dapr.Client/BulkPublishRequestEntry.cs Outdated
Comment thread src/Dapr.Client/BulkPublishRequestEntry.cs Outdated
Comment thread src/Dapr.Client/BulkPublishResponseFailedEntry.cs Outdated
Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bulk Pubsub implementation in SDK

4 participants