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

Allow gruf to receive request objects in call #191

Closed
dorner opened this issue Nov 16, 2023 · 2 comments · Fixed by #193
Closed

Allow gruf to receive request objects in call #191

dorner opened this issue Nov 16, 2023 · 2 comments · Fixed by #193

Comments

@dorner
Copy link
Contributor

dorner commented Nov 16, 2023

What? Why?

Currently, gruf only supports hashes when sending messages - it won't allow you to actually pass in the request object itself. E.g. in client_spec.rb, this doesn't work:

    subject { client.call(method_name, req, metadata, opts) }

We'd really like to use the request object directly because we can make use of Sorbet generated types when we do this. We also can't just call to_h on the object and pass it in, because there are bugs in this method (such as this one).

Other Notables

It seems like this would be simple enough to implement by just checking to see if the passed in params respond to the to_proto message and if so, not to call the request_object method and just use it as is.

I can have a quick PR with this up if there are no objections.

@splittingred
Copy link
Member

@dorner I think this is a reasonable request - and I agree generally with the approach.

Go for it on the PR - feel free to ping if you need assistance.

@dorner
Copy link
Contributor Author

dorner commented Nov 28, 2023

Thanks - here it is! #193 Let me know if I need to make any changes.

splittingred added a commit that referenced this issue Nov 29, 2023
#191. Allow passing of request object directly
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 a pull request may close this issue.

2 participants