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

Misleading comment in the Invoke example #93

Open
NickLarsenNZ opened this issue Apr 14, 2023 · 3 comments
Open

Misleading comment in the Invoke example #93

NickLarsenNZ opened this issue Apr 14, 2023 · 3 comments
Labels
documentation Improvements or additions to documentation P2 pinned dapr-bot exemption
Milestone

Comments

@NickLarsenNZ
Copy link
Contributor

NickLarsenNZ commented Apr 14, 2023

Looks like it was copy/pasted from the pubsub example.

Topic A Is not returned here, only the default (which is an empty list, given Vec is Default over T):

/// Lists all topics subscribed by this app.
///
/// NOTE: Dapr runtime will call this method to get
/// the list of topics the app wants to subscribe to.
/// In this example, the app is subscribing to topic `A`.
async fn list_topic_subscriptions(
&self,
_request: Request<()>,
) -> Result<Response<ListTopicSubscriptionsResponse>, Status> {
let list_subscriptions = ListTopicSubscriptionsResponse::default();
Ok(Response::new(list_subscriptions))
}

Perhaps it should read something like:

/// Return an empty list of topics. 

... but on that note, should the trait methods provide these default behaviours so we don't have to implement defaults for each unused method?

@NickLarsenNZ
Copy link
Contributor Author

NickLarsenNZ commented Apr 14, 2023

should the trait methods provide these default behaviours so we don't have to implement defaults for each unused method?

I'm not sure it is easily possible, since the trait is generated from proto files.
I'm unaware of a way to provide default implementations for trait methods generated this way.

tonic::include_proto!("dapr.proto.runtime.v1");


Update: I've asked the question over here: tokio-rs/prost#844

@mikeee mikeee added documentation Improvements or additions to documentation P2 labels Mar 20, 2024
@mikeee mikeee added this to the 1.14 milestone Mar 20, 2024
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted) or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 19, 2024
Copy link

This issue has been automatically closed because it has not had activity in the last 67 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue or help wanted. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
@mikeee mikeee added pinned dapr-bot exemption and removed stale labels May 27, 2024
@mikeee mikeee reopened this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation P2 pinned dapr-bot exemption
Projects
None yet
Development

No branches or pull requests

2 participants