🐛 Update typed Applyconfigurations with server response#3475
🐛 Update typed Applyconfigurations with server response#3475alvaroaleman wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
Prior to this, we would never update a typed applyconfiguration with the response from the server, this change fixes that.
k8s-ci-robot
added
approved
|
One question is if we should backport this, the problem with that is that it may break existing code, since the response contains a resource version, so when re-using an object to do multiple requests that would have previously worked but won't anymore after this |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. DetailsGit tree hash: 6568e3587eae1c7ea853eeeecea75a2f32d1d70d |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, dongjiang1989 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold We should discuss this a bit |
k8s-ci-robot
added
the
do-not-merge/hold
| return err | ||
| } | ||
|
|
||
| return json.Unmarshal(body, obj) |
There was a problem hiding this comment.
Do we have to do some sort of zeroing on the obj?
|
|
||
| err = cl.Apply(ctx, obj, &client.ApplyOptions{FieldManager: "test-manager"}) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(obj.ResourceVersion).NotTo(BeNil()) |
There was a problem hiding this comment.
Can we add something similar for subresource Apply?
Added my take on the issue |
4 participants
Prior to this, we would never update a typed applyconfiguration with the response from the server, this change fixes that.
Fixes #3474