-
Notifications
You must be signed in to change notification settings - Fork 65
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
Created Procedural Macros For Pub Sub #87
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice!
@zedgell you'll need to amend the commits with the DCO signoff. To reduce the noise in the target branch, it is a good idea to do an interactive rebase locally to tidy up the branch. To do so, git fetch
git rebase -i origin/main You should see something similar to:
In your editor, change the first word on each commit from An example is fixing up a typo from the parent commit:
Save, then
|
739e188
to
b8f28d7
Compare
@NickLarsenNZ Should be good now. Let me know if any other changes are needed |
b8f28d7
to
b44d390
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had more of a look over the code, and noted a couple of things.
Oops, I hit Approve instead of Comment
proc-macros/src/lib.rs
Outdated
struct #struct_name_ident; | ||
|
||
#[tonic::async_trait] | ||
impl AppCallback for #struct_name_ident { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to use fully qualified names to avoid name collision.
proc-macros/src/lib.rs
Outdated
let topic = #topic.to_string(); | ||
let pubsub_name = #pub_sub_name.to_string(); | ||
|
||
let list_subscriptions = ListTopicSubscriptionsResponse::topic(pubsub_name, topic); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this would only work for a single topic.
The ListTopicSubscriptionsResponse::topic
method is a bit misleading. See #81 (comment) to see how to add multiple topics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, my bad, I see each use of the macro creates an AppCallback
implementation.
As far as I could see, this doesn't work with tonic (see my other comment here: #81 (comment)).
So I think we need to have a single implementation generated, but I don't know how that can work in this case.
Could something like this work?
struct MyService {}
#[dapr] // with this creating the single AppCallback implementation
impl MyService {
// The pubsub_name and topic would be stored, and the method would then be used in the `on_topic_event` trait implementation (eg: using match).
// perhaps the function signature should have a parameter for pubsub name and topic (eg a Context struct)
#[subscribe(pubsub_name = "pubsub", topic = "A")]
async fn handle_message(&self, order: Order) {
println!("{:#?}", order)
}
}
I changed a couple of things in the macro usage (based on the python-sdk):
topic
->subscribe
pub_sub_name
- >pubsub_name
Keen to hear your thoughts?
@NickLarsenNZ I will take a look over them. |
Hi @zedgell, did you manage to get any further? |
@NickLarsenNZ I added a potential fix for the multiple events. I wanted them to be able to define multiple handlers for each event but not have to create a new struct for each one. I looked at the way you suggested but that poses a challenge of trying to find that method and hoping they dont have one with a similar name. What I did was Created a central #[derive(Serialize, Deserialize, Debug)]
struct Order {
pub order_number: i32,
pub order_details: String,
}
#[topic(pub_sub_name = "pubsub", topic = "A")]
async fn handle_a_event(order: Order) {
println!("Topic A - {:#?}", order)
}
#[topic(pub_sub_name = "pubsub", topic = "B")]
async fn handle_b_event(order: Order) {
println!("Topic B - {:#?}", order)
}
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let addr = "127.0.0.1:50051".parse().unwrap();
let mut callback_service = AppCallbackService::new();
callback_service.add_handler(HandleAEvent::default().get_handler());
callback_service.add_handler(HandleBEvent::default().get_handler());
println!("AppCallback server listening on: {}", addr);
// Create a gRPC server with the callback_service.
Server::builder()
.add_service(AppCallbackServer::new(callback_service))
.serve(addr)
.await?;
Ok(())
} |
@NickLarsenNZ did you get a chance to review the above? |
ef8cea0
to
7fae989
Compare
Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: zachary <zacharyedgell@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* Update dependencies Signed-off-by: Hauke Jung <hauke.jung@outlook.de> * Add grpc invoke examples Signed-off-by: Hauke Jung <hauke.jung@outlook.de> * Fix fmt errors Signed-off-by: Hauke Jung <hauke.jung@outlook.de> * Install protoc on github workflow Signed-off-by: Hauke Jung <hauke.jung@outlook.de> --------- Signed-off-by: Hauke Jung <hauke.jung@outlook.de> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* workflow - install protoc Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> * pinning protoc version to 3.21.12 Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> * trying a new version Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> * workflow - fixes rust toolchain and protoc installs Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> --------- Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* updates grpc proto for dapr v1.10 release Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> * updates dependencies to most recent Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> --------- Signed-off-by: Roberto J Rojas <robertojrojas@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: joshvanl <me@joshvanl.dev> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: joshvanl <me@joshvanl.dev> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* Updating to dapr runtime - master Signed-off-by: Elena Kolevska <elena@kolevska.com> * Updates the sdk version in the README. Gives the rest of the instructions in the file some love. Signed-off-by: Elena Kolevska <elena@kolevska.com> * Fixes typo Signed-off-by: Elena Kolevska <elena@kolevska.com> --------- Signed-off-by: Elena Kolevska <elena@kolevska.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* Update proto Signed-off-by: Cyril Scetbon <cscetbon@gmail.com> * Use v1.12.4 instead of master for now Signed-off-by: Cyril Scetbon <cscetbon@gmail.com> --------- Signed-off-by: Cyril Scetbon <cscetbon@gmail.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
* Updates protos from release-1.13.0 Signed-off-by: Elena Kolevska <elena@kolevska.com> * Now from the correct version Signed-off-by: Elena Kolevska <elena@kolevska.com> * uses GetMetadataRequest Signed-off-by: Elena Kolevska <elena@kolevska.com> --------- Signed-off-by: Elena Kolevska <elena@kolevska.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: mikeee <hey@mike.ee> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: mikeee <hey@mike.ee> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: mikeee <hey@mike.ee> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: yaron2 <schneider.yaron@live.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Marc Duiker <marcduiker@users.noreply.github.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Caleb Cartwright <caleb.cartwright@outlook.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
Signed-off-by: Caleb Cartwright <caleb.cartwright@outlook.com> Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
ef8cea0
to
781b70d
Compare
# Conflicts: # .github/workflows/ci.yml # Cargo.toml # examples/pubsub/README.md # examples/pubsub/subscriber.rs # src/client.rs # src/lib.rs
Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
@mikeee Should be good now. Excuse all the extra commits. May have reverted a little to far the first time when I forgot to sign off lol. |
No dramas, thank you for picking this up so quickly after it being open for a long time! |
Signed-off-by: Zachary Edgell <zacharyedgell@gmail.com>
@mikeee looks like it ran fine now. Let me know if you see any other potential problems. |
Signed-off-by: Zachary K Edgell <zacharyedgell@gmail.com>
Signed-off-by: Mike Nguyen <hey@mike.ee>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, will pick up the failing pubsub validation in another issue/pr.
Added a Procedural Macro for pubsub client and updated publisher and subscriber example to demo the macro.