Add merge and json patch #390
Conversation
tsontario
force-pushed
the
add_merge_and_json_patch
branch
from
9a1db32 to
01093e3
Compare
There was a problem hiding this comment.
Great work ❤️ Some minor suggestions.
Could you add this to README, including explaining that .patch is strategic merge patch (not obvious from name), that it won't work for custom resources, and a link to https://kubernetes.io/docs/tasks/run-application/update-api-object-kubectl-patch/ and/or the format RFCs?
And mention in CHANGELOG please (prepend new "Unreleased" section if needed)
timothysmith0609
force-pushed
the
add_merge_and_json_patch
branch
from
9a9f80b to
c01378c
Compare
test/test_service.rb
Outdated
|
|
||
| patch = [ | ||
| { 'op' => 'replace', 'path' => '/metadata/annotations/key1', 'value' => 'value' }, | ||
| { 'op' => 'add', 'path' => '/metadata/annotations/key2', 'value' => 'value' } |
There was a problem hiding this comment.
in principle, once you use functionally different patch here, ideally you'd mock a different return result than service_patch.json. If you don't want that, nor want to patch only /metadata/annotations/key, please add a comment on the to_return saying it's inaccurate result for this patch.
There was a problem hiding this comment.
Mocked different resources and tested with against a minikube instance. Both merge_patch and json_patch are working as expected and as described in the tests now
|
Ok, this is ready for another round of review.
|
basic test. TODO model in correct behaviour of different patches refactor rubocop fixes Tidy up test police namespace mandatory CHANGELOG + README (incomplete) Revert namespace is mandatory separate fixtures for json + merge patch + assert headers in request README tweak Police, test bugfix, final README edit fix last bad test README typo
tsontario
force-pushed
the
add_merge_and_json_patch
branch
from
140c9c4 to
0f3ab7c
Compare
|
Squashed and ready to go. Thanks for the solid feedback and looking forward to the next PR. |
|
released in gem 4.3.0. |
Closes #357
Following the conversation on #357 and #268 , I figured I could start working on this feature as my first contribution to kubeclient. As @cben proposed, I'm simply adding
json_patch_*andmerge_patch_*methods to the client.