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

Support passing protobuf in text format #51

Closed
bluecmd opened this issue Sep 29, 2018 · 3 comments · Fixed by #54
Closed

Support passing protobuf in text format #51

bluecmd opened this issue Sep 29, 2018 · 3 comments · Fixed by #54

Comments

@bluecmd
Copy link

bluecmd commented Sep 29, 2018

Hi,

I'm using grpcurl as a library in a client CLI application. Since writing JSON lacks a bit of user friendliness, I would prefer to use the protobuf text format instead.

Compare:

grpcurl -d '{"fan": 0, "mode": "FAN_MODE_PERCENTAGE", "percentage": 50}' -plaintext 127.0.0.1:80 bmc.ManagementService.SetFan

with:

grpcurl -d 'fan: 0, mode: FAN_MODE_PERCENTAGE, percentage: 50' -plaintext 127.0.0.1:80 bmc.ManagementService.SetFan

or as it will be in the client I'm writing:

ubmcctl SetFan fan: 0, mode: FAN_MODE_PERCENTAGE, percentage: 50

Looking at the code I don't see any clear way of making this happen without breaking the interface, except for allowing JSON / Text format interchangeably. Which may be a feature, but could also create some confusion.

Thanks for your consideration

@bluecmd
Copy link
Author

bluecmd commented Oct 15, 2018

Ping @jhump, you seem to be the most active maintainer - do you have any suggestions here? I'd love to contribute a patch if needed, but not sure how you folks would like this done (if at all that is).

@jhump
Copy link
Contributor

jhump commented Oct 15, 2018

@bluecmd, sorry for the lack of response. I think I missed this in my inbox when you originally filed it.

I've started on a branch to address it (#54), but it's not ready to merge yet. I'll play around with it a little more, and add some new tests. After that, I think it will be ready to go.

@jhump jhump closed this as completed in #54 Oct 17, 2018
@bluecmd
Copy link
Author

bluecmd commented Oct 17, 2018

Great! Thanks :)

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