Any call that use entity config should symbolize its keys

[motymichaely]

Hey,

There's a potential issue with calls that use entity_config (like here). They all assume entity_config has symbolized keys.

I'd propose normalizing entity_config hash by symbolizing its keys:

def normalize_entity_config(entity_config)
  entity_config.to_hash.each_with_object({}) { |(k,v),o| o[k.to_sym] = v }
end

Any rejection for suggesting a pull request?

Thanks!

[simon3z]

cc @moolitayer

[moolitayer]

RecursiveOpenStruct symbolizes keys for us: "Also, by default it will turn all hash keys into symbols internally:"

pod = {
  'metadata' => 
...
client.create_pod(pod) # will not work
client.create_pod(Kubeclient::Pod.new(pod)) # will not work
client.create_pod(Kubeclient::Pod.new(POD)) # will work

We should not re implement rails' deep_symbolize_keys

[motymichaely]

@moolitayer Great. So I would suggest adding some info on this to the documentation.

For example:

namespace_str = <<-EOF
apiVersion: v1
kind: Namespace
metadata:
  name: "my-namespace"
  labels:
    name: "my-namespace"
EOF
client.create_namespace(YAML.load(namespace_yaml_str))

Produces this error:

NoMethodError: undefined method `[]' for nil:NilClass
    from /Users/moty/.rbenv/versions/2.0.0-p481/lib/ruby/gems/2.0.0/gems/kubeclient-1.2.0/lib/kubeclient/common.rb:229:in `create_entity'
    from /Users/moty/.rbenv/versions/2.0.0-p481/lib/ruby/gems/2.0.0/gems/kubeclient-1.2.0/lib/kubeclient/common.rb:116:in `block (2 levels) in define_entity_methods'

Based on your suggestion, this works:

client.create_namespace(Kubeclient::Namespace.new(YAML.load(namespace_str)))

[moolitayer]

@motymichaely I'm good with a Readme change. I think the best we can do is to consistently advocate the usage of the generic Kubeclient::resource class post #185 throughout the Readme as well as all the tests. There are just too many ways being used in our example code now and that is confusing as well as the problem described in this issue.

In your example:

client.create_namespace(Kubeclient::Resource.new(YAML.load(namespace_str)))

@abonas @simon3z agree/disagree?

[abonas]

@moolitayer :+1:

[moolitayer]

@motymichaely interested in submitting a pr as per the above comment ?

[grosser]

FYI Decided to go with .deep_symbolize_keys (rails) in our project... easier to debug and no more ObjectStruct bugs/overhead ...

[cben]

Now that we have as: :parsed_symbolized and as: :parsed (#306), kubeclient get_foo may give you data that isn't a Kubeclient::Resource, and in the latter case that you can't write back with create_foo/update_foo. Also, now that we use a single Kubeclient::Resource class everywhere, there's less value in forcing users to wrap everything in it...

=> I don't have bandwidth to work on this but would welcome a PR. @moolitayer are you still against?

[grosser]

would prefer telling the user what they do wrong instead of doing work behind their back -> call .to_h on what they give in and check if first key is a symbol, otherwise ArgumentError

[cben]

hmm, good idea.