support parsed json and parsed json with symbolized keys #306
Conversation
lib/kubeclient/common.rb
Outdated
| body | ||
| when :parsed | ||
| result = JSON.parse(body) | ||
| result = result.fetch('items') if list_type |
There was a problem hiding this comment.
I'm reluctant to drop information, especially collection resource_version. "Use this format, unless you need that one info it doesn't support"...
Simplest alternative is just return top-level data, but I agree requiring user to access [:items] for common case is cumbersome (and inconsistent with :ros behavior).
How about returning an array that also answers .kind and .resource_version? One way to implement:
module EntityListMixin
attr_accessor :kind, :resource_version
end
result = JSON.parse(body)
if list_type
result.extend(EntityListMixin)
result.kind = list_type
result.resource_version = resource_version
end
P.S. opened #307 to discuss some funkiness in how we set collection .kind. That is orthogonal, but there is one aspect that might be relevant here:
- I'd love
get_foosin the future to populate .kind on each entity in the array, not just on the collection. The k8s API is IMHO unfriendly here, it only gives you"kind": "NodeList"and assumes you're a strongly-typed client that knowsNodeList.itemsis an array ofNode.
There was a problem hiding this comment.
why would anyone need the kind/version of something they just queried ?
There was a problem hiding this comment.
kind is quite redundant but collection resource_version is useful to start a watch from.
Anyway, this can be added later.
There was a problem hiding this comment.
Note that the ListMeta object that you are discarding contains information that may be very important for certain applications, in particular the Continue attribute that is necessary for paging.
Also the Kind field may be very important for applications that try to process results in a generic way, explicitly checking the type of each object to decide what to do.
All in all I'd say that removing the step to get [:items] isn't really worth.
There was a problem hiding this comment.
Continueis never returned fromroseither ?Kindis always what the user requested anyway, so idk if there are a lot of usecases that need that
... changed it back since fetching items is easy and means we have less code/feature-requests ...
README.md
Outdated
| @@ -304,6 +304,8 @@ pods = client.get_pods as: :raw | |||
| node = client.get_node "127.0.0.1", as: :raw | |||
| ``` | |||
|
|
|||
| Other formats are: `:parsed` for `JSON.parse` or `:parsed_symbolized` for `JSON.parse(..., symbolize_names: true)` | |||
|
|
|||
There was a problem hiding this comment.
Could you include :ros also in the supported formats ?
Before this PR, any type other than :raw was considered as :ros(default response format), or even if miss-typed (couldn't find any validation for the expected :as value).
However, with this PR, it is mandatory now for clients to specify exactly :ros or not to pass a format in order to get response it in that format.
grosser
force-pushed
the
grosser/format
branch
from
70e237e
to
fd6aedf
Compare
grosser
force-pushed
the
grosser/format
branch
from
fd6aedf
to
4da6d9f
Compare
@cben