-
Notifications
You must be signed in to change notification settings - Fork 11
Update README #62
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
base: master
Are you sure you want to change the base?
Update README #62
Conversation
cgay
left a comment
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.
Thanks for the PR!
| (defun main () | ||
| (grpc:init-grpc) | ||
| (grpc::run-grpc-proto-server "localhost:8080" 'cl-protobufs.testing:greeter)) |
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.
Looks like we should be exporting run-grpc-server and run-grpc-proto-server. Feel like making another PR? :)
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.
Happy to go do that.
| ``` | ||
| ; load the protobuf-generated code here | ||
| (defmethod cl-protobufs.testing-rpc::say-hello ((request cl-protobufs.testing:hello-request) call) |
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.
I believe say-hello is exported.
The package names don't look right to me. Assuming the server example uses the same :local-nicknames as those in client-insecure.lisp it looks like it should be this instead:
(defmethod testing-rpc:say-hello ((request testing:hello-request) call)
but I haven't actually tried it. WDYT?
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.
I opted to use the fully-qualified names since someone who is testing this in the REPL would have to use those (as opposed to local-nicknames as part of a defsystem. Let me know your preference and I'll go double check that say-hello is exported.
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.
Aha, you've been looking at examples/client/client-insecure.lisp which uses examples/client/helloworld.proto which defines package lisp.grpc.integration_testing. However, I've been looking a few lines up in the README, which defines package testing. This is why we disagree about the namespace.
I do think there is value in providing a minimum working example so if there are no objections I'll add that.
|
I think with your other PR merged you can now just reference the exported symbols here and we're good to go? |
|
Almost; thanks for poking on this. I have an example sitting around locally with a demo server for |
|
freindly ping? |
|
Pong! I'll get to it soon:tm:! :') |
Although server support was added some time ago, the README hasn't been updated to reflect this.
While here, add some basic instructions for installing this library.
Also add a minimal example to demonstrate running a gRPC server.