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

Subscriptions: bi-directional subscription streaming. #7747

Merged
merged 11 commits into from
May 28, 2024

Conversation

JoshVanL
Copy link
Contributor

Adds SubscribeTopicEvents proto API which dynamically subscribes to pubsub topics based on dapr/proposals#52.

This is a basic gRPC implementation of the API whereby, like Subscription hot-reloading today, subscribing to a topic will reload every active subscription for the current daprd. In a future PR, reloading of Subscriptions will be granular to the specific pubsub topic.

Stream subscriptions are also only active once daprd declares the application as both present and ready. Dynamic stream subscriptions should be active both whether a app is running or not, as well as whether it is ready or not. This will be addressed in a future PR.

Adds SubscribeTopicEvents proto API which dynamically subscribes to
pubsub topics based on dapr/proposals#52.

This is a basic gRPC implementation of the API whereby, like
Subscription hot-reloading today, subscribing to a topic will reload
_every_ active subscription for the current daprd. In a future PR,
reloading of Subscriptions will be granular to the specific pubsub
topic.

Stream subscriptions are also only active once daprd declares the
application as both present and ready. Dynamic stream subscriptions
should be active both whether a app is running or not, as well as
whether it is ready or not. This will be addressed in a future PR.

Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL JoshVanL changed the title Subscriptions: bi-directional subscription & publish streaming. Subscriptions: bi-directional subscription streaming. May 22, 2024
Copy link

codecov bot commented May 22, 2024

Codecov Report

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

Project coverage is 57.01%. Comparing base (091a204) to head (3da8ef4).
Report is 6 commits behind head on master.

Current head 3da8ef4 differs from pull request most recent head 68d520a

Please upload reports for the commit 68d520a to get more accurate results.

Files Patch % Lines
pkg/runtime/pubsub/subscriptions.go 0.00% 112 Missing ⚠️
pkg/runtime/processor/pubsub/streamer.go 6.60% 99 Missing ⚠️
pkg/runtime/processor/pubsub/streamerconn.go 0.00% 17 Missing ⚠️
pkg/api/grpc/subscribe.go 0.00% 15 Missing ⚠️
pkg/runtime/processor/pubsub/publish.go 88.00% 2 Missing and 1 partial ⚠️
pkg/runtime/processor/pubsub/topics.go 80.00% 1 Missing and 2 partials ⚠️
pkg/runtime/processor/pubsub/bulk_subscriber.go 77.77% 2 Missing ⚠️
pkg/runtime/processor/pubsub/pubsub.go 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7747      +/-   ##
==========================================
- Coverage   61.39%   57.01%   -4.38%     
==========================================
  Files         265      477     +212     
  Lines       22609    25809    +3200     
==========================================
+ Hits        13880    14716     +836     
- Misses       7579     9929    +2350     
- Partials     1150     1164      +14     

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

Signed-off-by: joshvanl <me@joshvanl.dev>
hot-op-inf-comp test

Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL JoshVanL marked this pull request as ready for review May 23, 2024 00:17
@JoshVanL JoshVanL requested review from a team as code owners May 23, 2024 00:17
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2024

Codecov Report

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

Project coverage is 57.01%. Comparing base (091a204) to head (3da8ef4).
Report is 8 commits behind head on master.

Current head 3da8ef4 differs from pull request most recent head 7204427

Please upload reports for the commit 7204427 to get more accurate results.

Files Patch % Lines
pkg/runtime/pubsub/subscriptions.go 0.00% 112 Missing ⚠️
pkg/runtime/processor/pubsub/streamer.go 6.60% 99 Missing ⚠️
pkg/runtime/processor/pubsub/streamerconn.go 0.00% 17 Missing ⚠️
pkg/api/grpc/subscribe.go 0.00% 15 Missing ⚠️
pkg/runtime/processor/pubsub/publish.go 88.00% 2 Missing and 1 partial ⚠️
pkg/runtime/processor/pubsub/topics.go 80.00% 1 Missing and 2 partials ⚠️
pkg/runtime/processor/pubsub/bulk_subscriber.go 77.77% 2 Missing ⚠️
pkg/runtime/processor/pubsub/pubsub.go 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7747      +/-   ##
==========================================
- Coverage   61.39%   57.01%   -4.38%     
==========================================
  Files         265      477     +212     
  Lines       22609    25809    +3200     
==========================================
+ Hits        13880    14716     +836     
- Misses       7579     9929    +2350     
- Partials     1150     1164      +14     

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

dapr/proto/runtime/v1/dapr.proto Outdated Show resolved Hide resolved
pkg/api/grpc/endpoints.go Outdated Show resolved Hide resolved
pkg/proto/runtime/v1/dapr_tracing.go Show resolved Hide resolved
Signed-off-by: joshvanl <me@joshvanl.dev>
Copy link
Member

@daixiang0 daixiang0 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 yaron2 merged commit 7093021 into dapr:master May 28, 2024
18 of 20 checks passed
elena-kolevska pushed a commit to elena-kolevska/dapr that referenced this pull request Jun 10, 2024
* Subscriptions: bi-directional subscription & publish streaming.

Adds SubscribeTopicEvents proto API which dynamically subscribes to
pubsub topics based on dapr/proposals#52.

This is a basic gRPC implementation of the API whereby, like
Subscription hot-reloading today, subscribing to a topic will reload
_every_ active subscription for the current daprd. In a future PR,
reloading of Subscriptions will be granular to the specific pubsub
topic.

Stream subscriptions are also only active once daprd declares the
application as both present and ready. Dynamic stream subscriptions
should be active both whether a app is running or not, as well as
whether it is ready or not. This will be addressed in a future PR.

Signed-off-by: joshvanl <me@joshvanl.dev>

* Updates go.mod go version to 1.22.3 in e2e apps

Signed-off-by: joshvanl <me@joshvanl.dev>

* Remove small context timeout on httpserver int tests

Signed-off-by: joshvanl <me@joshvanl.dev>

* Wait for daprd2 to be ready before calling meta endpoint in
hot-op-inf-comp test

Signed-off-by: joshvanl <me@joshvanl.dev>

* Increase int test daprd wait until ready timeout to 30s

Signed-off-by: joshvanl <me@joshvanl.dev>

* Assert httpendpoint int test resp body with eventually

Signed-off-by: joshvanl <me@joshvanl.dev>

* Set subscription APIs Alpha1

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
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