initial bulk publish impl for java by mukundansundar · Pull Request #789 · dapr/java-sdk

mukundansundar · 2022-09-26T12:20:17Z

Signed-off-by: Mukundan Sundararajan [email protected]

Description

initial bulk publish implementation for Java SDK

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.
Since bulk subscribe is already implemented, this PR closes #778

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

Signed-off-by: Mukundan Sundararajan <[email protected]>

artursouza · 2022-09-27T16:56:02Z

+        String contentType = entry.getContentType();
+
+        // Serialize event into bytes
+        if (!Strings.isNullOrEmpty(contentType) && objectSerializer instanceof DefaultObjectSerializer) {


This is a problem we need to fix in other places in the SDK. Using the default serializer should not trigger different behavior. It should be based on the content-type only.

this is a change that needs to be done SDK wide and not only for one api ... If it is is only changed here that might cause serialized events from this API to become incompatible with the other normal publish API ...

johnewart

Just a few questions / observations, mostly for my own edification - you don't need to block on them

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar · 2022-09-29T06:27:56Z

The build failures here are wrt Config API changes which is handled in the PR #740.

Locally also verified that the PubsubIT passes as expected ....
cc @dapr/maintainers-java-sdk

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar · 2023-01-06T13:40:07Z

As discussed offline, this PR only contains the gRPC implementation for Bulk Publish

artursouza · 2023-01-09T04:54:20Z

+   */
+  @Override
+  public <T> Mono<BulkPublishResponse<T>> publishEvents(BulkPublishRequest<T> request) {
+    return DaprException.wrapMono(new UnsupportedOperationException());


I assume this will be implemented next, right?

@artursouza as we discussed previously, we were saying that HTTP impl is not needed, and that HTTP Client is going to be deprecated. #790

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar · 2023-01-11T16:02:00Z

@dapr/maintainers-java-sdk see the latest checks for this PR itself. It required only 2 runs.
The older one https://github.com/dapr/java-sdk/actions/runs/3871888965 required 4 attempts. (Before adding doc content)

For some reason the Kafka subscription is flaky.

Additionally I am seeing flakiness in other validations as well. Please see the logs

mukundansundar · 2023-01-13T06:34:21Z

@artursouza any updates on this PR?

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar · 2023-01-18T07:06:42Z

@dapr/maintainers-java-sdk see the latest checks for this PR itself. It required only 2 runs. The older one https://github.com/dapr/java-sdk/actions/runs/3871888965 required 4 attempts. (Before adding doc content)

For some reason the Kafka subscription is flaky.

Additionally I am seeing flakiness in other validations as well. Please see the logs

Changed the examples to use Redis, that removes the flakiness.

Signed-off-by: Mukundan Sundararajan <[email protected]>

codecov · 2023-01-19T04:49:36Z

Codecov Report

Merging #789 (99639df) into master (b83661d) will increase coverage by 0.35%.
The diff coverage is 67.56%.

@@             Coverage Diff              @@
##             master     #789      +/-   ##
============================================
+ Coverage     77.62%   77.97%   +0.35%     
- Complexity     1161     1258      +97     
============================================
  Files           105      116      +11     
  Lines          3647     3887     +240     
  Branches        419      457      +38     
============================================
+ Hits           2831     3031     +200     
- Misses          603      625      +22     
- Partials        213      231      +18

Impacted Files	Coverage Δ
.../client/domain/BulkPublishResponseFailedEntry.java	`0.00% <0.00%> (ø)`
...o/dapr/client/domain/BulkSubscribeAppResponse.java	`0.00% <0.00%> (ø)`
...r/client/domain/BulkSubscribeAppResponseEntry.java	`0.00% <0.00%> (ø)`
.../client/domain/BulkSubscribeAppResponseStatus.java	`0.00% <0.00%> (ø)`
...va/io/dapr/client/domain/BulkSubscribeMessage.java	`0.00% <0.00%> (ø)`
.../dapr/client/domain/BulkSubscribeMessageEntry.java	`0.00% <0.00%> (ø)`
sdk/src/main/java/io/dapr/utils/TypeRef.java	`78.33% <44.44%> (-5.99%)`	⬇️
...ava/io/dapr/client/domain/BulkPublishResponse.java	`50.00% <50.00%> (ø)`
...c/main/java/io/dapr/client/AbstractDaprClient.java	`84.76% <55.55%> (-2.74%)`	⬇️
...n/java/io/dapr/client/domain/BulkPublishEntry.java	`58.82% <58.82%> (ø)`
... and 14 more

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

mukundansundar · 2023-01-19T05:04:04Z

@dapr/maintainers-java-sdk Had a discussion with @shubham1172 on whether to merge BulkSubscribeMessageEntry and BulkPublishEntry classes into a single class or not. As of now what we feel is that it is clearer to have two different set of classes for BulkPublish and BulkSubscribe and not have a common class in-between. There is some duplication, but since the APIs are orthogonal they can be used independently of each other and it will be clearer if we have two sets of classes and not have a common BulkMessageEntry class which will be used by BulkPublish and BulkSubscribe APIs.

initial bulk publish impl for java

db5a915

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar commented Sep 26, 2022

View reviewed changes

Comment thread pom.xml Outdated

mukundansundar commented Sep 26, 2022

View reviewed changes

Comment thread sdk/pom.xml Outdated

mukundansundar mentioned this pull request Sep 26, 2022

Proposal: Support batching of send and receive pub/sub messages for performance dapr/dapr#2218

Closed

mukundansundar added 3 commits September 26, 2022 17:52

Merge branch 'master' into bulk-publish-sdk

2c08948

add UTs and clean up java doc for new interface methods.

5bbbaaa

Signed-off-by: Mukundan Sundararajan <[email protected]>

add more interface methods for bulk publish

7879e31

Signed-off-by: Mukundan Sundararajan <[email protected]>

artursouza reviewed Sep 27, 2022

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/DaprClientHttp.java Outdated

artursouza reviewed Sep 27, 2022

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/DaprClientHttp.java Outdated

johnewart reviewed Sep 27, 2022

View reviewed changes

adding examples and ITs for bulk publish

c03cd4c

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar marked this pull request as ready for review September 28, 2022 20:42

mukundansundar requested review from a team as code owners September 28, 2022 20:42

mukundansundar added 4 commits September 29, 2022 02:26

addressing review comments

93d4b99

Signed-off-by: Mukundan Sundararajan <[email protected]>

use latest ref from dapr branch

8bd8b33

Signed-off-by: Mukundan Sundararajan <[email protected]>

add example validation

fcd33cc

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix bindings example validation

3e8ae6c

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar mentioned this pull request Nov 25, 2022

[EPIC] Make Bulk Pubsub APIs stable dapr/dapr#5567

Closed

64 tasks

mukundansundar added 6 commits December 16, 2022 10:33

Merge branch 'master' into bulk-publish-sdk

ad66e82

make changes for latest bulk publish dapr changes

2a5c50d

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix examples

c37ebe0

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix examples

28218b3

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix typo

c423e40

Signed-off-by: Mukundan Sundararajan <[email protected]>

test against java 11 only

48b3a6c

Signed-off-by: Mukundan Sundararajan <[email protected]>

yash-nisar reviewed Dec 23, 2022

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/domain/BulkPublishResponse.java Outdated

yash-nisar reviewed Dec 23, 2022

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/DaprClientGrpc.java Outdated

yash-nisar reviewed Dec 23, 2022

View reviewed changes

Comment thread examples/src/main/java/io/dapr/examples/pubsub/bulk/BulkPublisher.java Outdated

mukundansundar added 3 commits January 6, 2023 16:16

change match order in examples

481ed34

Signed-off-by: Mukundan Sundararajan <[email protected]>

set fail fast as false

9bc9a5c

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix Internal Invoke exception in ITs

84f3273

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar commented Jan 6, 2023

View reviewed changes

Comment thread sdk-tests/src/test/java/io/dapr/it/methodinvoke/grpc/MethodInvokeIT.java Outdated

artursouza suggested changes Jan 9, 2023

View reviewed changes

mukundansundar added 4 commits January 9, 2023 12:28

address review comments

59dd45d

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix IT

2241d7c

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix app scopes in examples

a9807d9

Signed-off-by: Mukundan Sundararajan <[email protected]>

add content to daprdocs

546818b

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar mentioned this pull request Jan 12, 2023

Bulk Pubsub implementation in SDK dapr/python-sdk#464

Closed

shubham1172 mentioned this pull request Jan 13, 2023

feat(pubsub): add bulk subscribe support #791

Merged

3 tasks

artursouza suggested changes Jan 13, 2023

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/client/domain/BulkPublishResponse.java Outdated

Comment thread sdk/src/main/java/io/dapr/client/domain/BulkPublishRequest.java Outdated

artursouza suggested changes Jan 13, 2023

View reviewed changes

Comment thread examples/components/pubsub/kafka.yaml Outdated

artursouza and others added 4 commits January 17, 2023 18:32

Merge remote-tracking branch 'upstream/master' into bulk-publish-sdk

a822a7a

address review comments

6a5e962

Signed-off-by: Mukundan Sundararajan <[email protected]>

fix mm.py step comment

0a98ed1

Signed-off-by: Mukundan Sundararajan <[email protected]>

reset bindings examples readme

8aaba36

Signed-off-by: Mukundan Sundararajan <[email protected]>

mukundansundar requested a review from artursouza January 18, 2023 07:11

shubham1172 mentioned this pull request Jan 19, 2023

Bulk PubSub implementation #778

Closed

mukundansundar added 2 commits January 19, 2023 09:34

Merge branch 'master' into bulk-publish-sdk

89daf58

fix example, IT and make classes immutable

99639df

Signed-off-by: Mukundan Sundararajan <[email protected]>

artursouza approved these changes Jan 19, 2023

View reviewed changes

artursouza merged commit 81591b9 into dapr:master Jan 19, 2023

mukundansundar deleted the bulk-publish-sdk branch January 19, 2023 21:01

Uh oh!

Conversation

mukundansundar commented Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

Uh oh!

Uh oh!

artursouza Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

mukundansundar Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

johnewart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukundansundar commented Sep 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukundansundar commented Jan 6, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artursouza Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

mukundansundar Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukundansundar commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mukundansundar commented Jan 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukundansundar commented Jan 18, 2023

Uh oh!

codecov Bot commented Jan 19, 2023

Codecov Report

Uh oh!

mukundansundar commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mukundansundar commented Sep 26, 2022 •

edited

Loading

mukundansundar Sep 29, 2022 •

edited

Loading

mukundansundar commented Jan 11, 2023 •

edited

Loading

mukundansundar commented Jan 19, 2023 •

edited

Loading