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

Ruby: Allow for the instantiation of nested message fields with ruby-hashes #3120

Closed
landrito opened this issue May 22, 2017 · 13 comments · Fixed by #5716
Closed

Ruby: Allow for the instantiation of nested message fields with ruby-hashes #3120

landrito opened this issue May 22, 2017 · 13 comments · Fixed by #5716

Comments

@landrito
Copy link

What

Currently instantiation of a protobuf message can be done using a ruby-hash but does not support instantiating the nested messages using nested hashes. Being able to support the instantiation of messages including nested messages using a single hash would be a big usability win for googleapis/toolkit.

Example

Current

message = MyMessage.new({
  submessage_field: SubMessage.new(:foo => 42)
})

Requested

message = MyMessage.new({
  submessage_field: {foo: 42}
})
@landrito
Copy link
Author

cc @geigerj @lukesneeringer

@acozzette
Copy link
Member

CC @haberman
That sounds like a useful feature; I'm not sure if anyone on the proto team has the cycles to work on this but feel free to send us a pull request if you want to try implementing it.

@geigerj
Copy link

geigerj commented Jun 1, 2017

@acozzette We have limited bandwidth for the remainder of the quarter, but this is an important usability problem for the Ruby Cloud client libraries. Could you send us some code links so we can get some sense what changes would be needed to the protobuf codebase to enable this?

(If we're not able to make the fix in protobuf, we might work around by providing a utility method in the client that transforms hashes to protobuf messages. But if possible, I'd prefer to fix this here rather than in the client libraries.)

@acozzette
Copy link
Member

@geigerj I'm not actually that familiar with the protobuf implementation for Ruby. @haberman, maybe you could comment on this?

@geigerj
Copy link

geigerj commented Jun 5, 2017

@haberman -- could you advise here?

@geigerj
Copy link

geigerj commented Dec 7, 2017

@zanker @acozzette It looks like this was implemented in #3627, in which case this issue could be closed. But I'm having trouble getting the feature to work in protobuf 3.5.0:

> require 'google/protobuf/struct_pb'
 => true 
> s = Google::Protobuf::Struct.new(fields: {'a' => {number_value: 0.0}})
TypeError: Invalid type Hash to assign to submessage field.

@zanker -- is this the intended usage of the feature you implemented in #3627?

@zanker
Copy link
Contributor

zanker commented Dec 7, 2017

Hm, that's odd. That is the correct syntax, and the test for it https://github.com/google/protobuf/pull/3627/files#diff-bbb59c88129c0bf6537d0487634b282dR205 is the same as what you have, and it does look like the code made it into 3.5.0.

Silly question, but are you sure you're pulling 3.5.0? That was something I had running on Square code before I left, so the feature does work.

@geigerj
Copy link

geigerj commented Dec 7, 2017

Yep, it's 3.5.0:

> Gem.loaded_specs['google-protobuf'].version.version
 => "3.5.0" 

@antonioparisi
Copy link

@geigerj any luck with this?

@geigerj
Copy link

geigerj commented Mar 2, 2018

@antonioparisi I am not aware of any updates. Some of the others on this issue might be able to advise.

@antonioparisi
Copy link

@zanker ?

@premist
Copy link

premist commented Apr 27, 2018

Also experiencing this issue on 3.5.1.2.

pry(main)> Gem.loaded_specs['google-protobuf'].version.version
=> "3.5.1.2"
pry(main)> Google::Protobuf::Struct.new(fields: { "a" => { number_value: 0 } })
TypeError: Invalid type Hash to assign to submessage field.
from (pry):29:in `initialize'

@antonioparisi
Copy link

any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants