Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make PATCH on par with PUT + regression fix #146

Merged
merged 3 commits into from
May 29, 2024
Merged

Conversation

mbarnes
Copy link
Collaborator

@mbarnes mbarnes commented May 24, 2024

What this PR does

This makes PATCH requests on par with PUT (in fact I combined the PUT and PATCH handlers).

This also fixes a visibility validation regression introduced in #124. (See the commit message in the commit that fixes it for details.) I discovered this while testing PATCH requests, so I just addressed it here instead of a separate pull request.

Jira: No Jira, just circling back to a to-do from the initial frontend pass

Special notes for your reviewer

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

  • PR: The PR description is expressive enough and will help future contributors
  • Code: Write code that humans can understand and Keep it simple
  • Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
  • Upgrade: Impact of this change on upgrade flows was considered and addressed if required
  • Deployment: The deployment process was considered and addressed if required
  • Testing: New code requires new unit tests.
  • Documentation: Is the documentation updated? Either in the doc located in focus area, in the README or in the code itself.
  • Customers: Is this change affecting customers? Is the release plan considered?

@mbarnes mbarnes force-pushed the handle-patch branch 3 times, most recently from 839b1e1 to d9cddce Compare May 28, 2024 17:54
@mbarnes mbarnes changed the title Make PATCH on par with PUT, and other fixes Make PATCH on par with PUT + regression fix May 28, 2024
Copy link

Please rebase pull request.

Matthew Barnes added 3 commits May 29, 2024 10:01
- Use path segment values instead of parsing resource IDs when
  possible.

- log.slog output functions need a complete message, not a format
  string.  Use fmt.Sprintf.
Logic is mostly the same.  The only notable difference is PATCH
requests will not create a new cluster.
Side-effect of commit 0463ecb.

Embedding the generated resource struct in our own resource struct
caused a mismatch with the StructTagMap keys, because the embedded
generated.HcpOpenShiftClusterResource field was showing up in the
validateVisibility type recursion.

e.g. mapKey would be "HcpOpenShiftClusterResource.properties.spec"
     whereas the StructTagMap just has "properties.spec"

Work around by passing the embedded HcpOpenShiftClusterResource
field to api.ValidateVisibility.
@mbarnes mbarnes merged commit b55c730 into Azure:main May 29, 2024
5 checks passed
@mbarnes mbarnes deleted the handle-patch branch May 29, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants