Skip to content

🐛 Update typed Applyconfigurations with server response#3475

Merged
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
alvaroaleman:fixfix
Apr 30, 2026
Merged

🐛 Update typed Applyconfigurations with server response#3475
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
alvaroaleman:fixfix

Conversation

@alvaroaleman

@alvaroaleman alvaroaleman commented Mar 8, 2026

Copy link
Copy Markdown
Member

Prior to this, we would never update a typed applyconfiguration with the response from the server, this change fixes that.

Fixes #3474

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 8, 2026
@alvaroaleman

alvaroaleman commented Mar 8, 2026

Copy link
Copy Markdown
Member Author

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

dongjiang1989
dongjiang1989 approved these changes Mar 10, 2026
View reviewed changes

@dongjiang1989 dongjiang1989 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 10, 2026
@k8s-ci-robot

k8s-ci-robot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: 6568e3587eae1c7ea853eeeecea75a2f32d1d70d

@sbueringer

sbueringer commented Mar 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

/hold

We should discuss this a bit

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 10, 2026
sbueringer
sbueringer reviewed Mar 11, 2026
View reviewed changes
Comment thread pkg/client/typed_client.go
Comment thread pkg/client/client_test.go
@sbueringer

sbueringer commented Mar 11, 2026

Copy link
Copy Markdown
Member

We should discuss this a bit

Added my take on the issue

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2026
@alvaroaleman alvaroaleman mentioned this pull request Apr 18, 2026
Open
@matheuscscp

matheuscscp commented Apr 30, 2026

Copy link
Copy Markdown

Hey @sbueringer, sorry for the ping, looks like this is the last unresolved item for v0.24.0 according to the tracking issue. Can you please review? 🙏

sbueringer
sbueringer reviewed Apr 30, 2026
View reviewed changes

@sbueringer sbueringer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit

Comment thread pkg/client/typed_client.go
@alvaroaleman
🐛 Update typed Applyconfigurations with server response
3296f32 
Prior to this, we would never update a typed applyconfiguration with the
response from the server, this change fixes that.
@sbueringer

sbueringer commented Apr 30, 2026

Copy link
Copy Markdown
Member

Thank you very much!

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 30, 2026
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 30, 2026
@k8s-ci-robot

k8s-ci-robot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: 8512a044c4f7eed269f873973c8a829f532cbcb3

@k8s-ci-robot

k8s-ci-robot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, dongjiang1989, sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [alvaroaleman,sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit d3eaef3 into kubernetes-sigs:main Apr 30, 2026
7 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 6, 2026
Closed
@erikgb erikgb mentioned this pull request May 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbueringer sbueringer sbueringer left review comments

@joelanford joelanford Awaiting requested review from joelanford

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

@dongjiang1989 dongjiang1989 Awaiting requested review from dongjiang1989

Assignees

@sbueringer sbueringer

@dongjiang1989 dongjiang1989

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Apply using typed apply configurations doesn't deserialize the response into the passed object

5 participants

@alvaroaleman @k8s-ci-robot @sbueringer @matheuscscp @dongjiang1989