Adding consumer for uSubscription service #292
Conversation
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
4b4518a
to
20d3c56
Compare
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
20d3c56
to
38912eb
Compare
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
38912eb
to
cc64305
Compare
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
cc64305
to
80ee1f9
Compare
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
80ee1f9
to
692eb81
Compare
There was a problem hiding this comment.
Minor: I would move this to include/up-cpp/client/usubscription/v3/SubscriptionClient.h. I'm also tempted to replace "SubscriptionClient" with "Subscriber" or "Consumer" (the publish side would be "Publisher" or "Producer")
| /// | ||
| /// Like all L3 client APIs, the SubscriptionClient is a wrapper on top of the | ||
| /// L2 Communication APIs and USubscription service. | ||
| struct SubscriptionClient { |
There was a problem hiding this comment.
I'm tempted to call this Subscriber or Consumer since client is already in the namespace. Not 100% sure there yet. What do you think?
There was a problem hiding this comment.
Subscriber may cause confusion between L2 Subscriber just in context of writing and talking.
Might be better to call it Consumer unless Consumer might be used elsewhere too
| SubscriptionResponse getSubscriptionResponse() const { | ||
| return subscription_response_; | ||
| } |
There was a problem hiding this comment.
I'm not sure if this is really needed - we can probably handle the response internally and provide just a status outward. I'll need to think on it a little more.
There was a problem hiding this comment.
message SubscriptionResponse {
// Current status of the subscription
SubscriptionStatus status = 1;
// Any platform (uDevice) specific additional event delivered configuration
// information for how the subscriber should consume the published events.
EventDeliveryConfig config = 2;
// Subscription topic passed to SubscriptionRequest
uprotocol.v1.UUri topic = 3;
}
As SubscriptionResponse may contain configuration information which might be critically to consume information. I am not entirely sure tho
|
Code coverage report is ready! 📈
|
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
b8e7a61
to
14215c0
Compare
agosh01
changed the title
agosh01
force-pushed
the
feature/add_usubscription_client
branch
from
14215c0
to
18e9c02
Compare
This PR is to implement consumer api which act as L3 interface for the user to do subscribe request to uSubscription service
Note: Current uSubscription proto file is not usable by C++ as its missing build configuration. In order for this PR to work up-core-api needs to updated. See