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

Bidirectional Subscriptions #578

Merged
merged 8 commits into from
Jul 11, 2024
Merged

Bidirectional Subscriptions #578

merged 8 commits into from
Jul 11, 2024

Conversation

JoshVanL
Copy link
Contributor

@JoshVanL JoshVanL commented Jun 19, 2024

Adds support for bidirectional subscriptions to PubSubs. Adds two methods for subscribing- one using a callback and one using an imperative approach. Both giving support to different programming styles or use cases.

Adds example with tests.

Requires daprd built from dapr/dapr#7770

@JoshVanL JoshVanL requested review from a team as code owners June 19, 2024 15:19
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 2.58621% with 113 lines in your changes missing coverage. Please review.

Project coverage is 62.05%. Comparing base (27248ba) to head (32b4f4b).
Report is 18 commits behind head on main.

Files Patch % Lines
client/subscribe.go 0.00% 113 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
+ Coverage   58.04%   62.05%   +4.00%     
==========================================
  Files          55       53       -2     
  Lines        3568     3352     -216     
==========================================
+ Hits         2071     2080       +9     
+ Misses       1375     1151     -224     
+ Partials      122      121       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mikeee
mikeee previously approved these changes Jul 2, 2024
Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

Lgtm!

JoshVanL and others added 7 commits July 9, 2024 14:19
Adds support for bidirectional subscriptions to PubSubs. Adds two
methods for subscribing- one using a callback and one using an
imperative approach. Both giving support to different programming styles
or use cases.

Adds example with tests.

Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: Mike Nguyen <hey@mike.ee>
Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

lgtm!

@yaron2
Copy link
Member

yaron2 commented Jul 11, 2024

the validation step is failing

@mikeee
Copy link
Member

mikeee commented Jul 11, 2024

the validation step is failing

It fails as the validation is running the latest version of Dapr rather than the RC.

I've validated with RC2 and it's smooth as butter 🙂 we could set the validation workflow to 1.14.0-rc.2 on the release branch if that helps push this through.

@yaron2 yaron2 merged commit b7b90e3 into dapr:main Jul 11, 2024
16 of 18 checks passed
@yaron2
Copy link
Member

yaron2 commented Jul 11, 2024

Can't argue with "smooth as butter" :)

@marcduiker
Copy link
Contributor

@holopin-bot @JoshVanL Thanks Josh!

Copy link

holopin-bot bot commented Aug 15, 2024

Congratulations @JoshVanL, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/clzvb8tyf10350cmgajs8h4lq

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

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.

None yet

4 participants