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

fix: Partial resource update, no longer generate updateMask field #3093

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

csh995426531
Copy link
Contributor

@csh995426531 csh995426531 commented Dec 24, 2022

References to other Issues or PRs
Fixes #2988

Have you read the Contributing Guidelines?
Yes

Brief description of what is fixed or changed
When the request method is patch, and the http body is not "*", do not need to generate updateMask field to swagger document

@google-cla
Copy link

google-cla bot commented Dec 24, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Hi, thanks for this PR! This looks great, but we're gonna need a few more things before we can merge it.

  1. The protoc-gen-grpc-gateway must opt-in to the field mask behavior. We should add this opt in option here too, and keep it named consistently: allow_patch_feature=true. Please add a flag that enables this behavior and keep it disabled by default.
  2. We have a great docs page for the patch feature: https://github.com/grpc-ecosystem/grpc-gateway/blob/main/docs/docs/mapping/patch_feature.md. Please add a section on how to use this new option to this page.

Thanks!

@csh995426531
Copy link
Contributor Author

Hi, thanks for this PR! This looks great, but we're gonna need a few more things before we can merge it.

  1. The protoc-gen-grpc-gateway must opt-in to the field mask behavior. We should add this opt in option here too, and keep it named consistently: allow_patch_feature=true. Please add a flag that enables this behavior and keep it disabled by default.
  2. We have a great docs page for the patch feature: https://github.com/grpc-ecosystem/grpc-gateway/blob/main/docs/docs/mapping/patch_feature.md. Please add a section on how to use this new option to this page.

Thanks!

Thank you for your guidance, I have revised these contents

docs/docs/mapping/patch_feature.md Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/main.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Thanks, this is almost there!

protoc-gen-openapiv2/internal/genopenapi/template.go Outdated Show resolved Hide resolved
protoc-gen-openapiv2/main.go Outdated Show resolved Hide resolved
@csh995426531
Copy link
Contributor Author

I didn't change anything else, why does the make generate command start with an error when I run it locally?
image

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

LGTM

@johanbrandhorst johanbrandhorst merged commit 8f41e47 into grpc-ecosystem:main Feb 4, 2023
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

@johanbrandhorst
Copy link
Collaborator

That panic is interesting, but it's probably fixed in new versions of buf. If you want, we could update the buf version we use.

another-rex pushed a commit to google/osv.dev that referenced this pull request Mar 6, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[github.com/grpc-ecosystem/grpc-gateway/v2](https://togithub.com/grpc-ecosystem/grpc-gateway)
| require | patch | `v2.15.0` -> `v2.15.2` |
|
[google.golang.org/grpc/cmd/protoc-gen-go-grpc](https://togithub.com/grpc/grpc-go)
| require | minor | `v1.2.0` -> `v1.3.0` |

---

### Release Notes

<details>
<summary>grpc-ecosystem/grpc-gateway</summary>

###
[`v2.15.2`](https://togithub.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.15.2)

[Compare
Source](https://togithub.com/grpc-ecosystem/grpc-gateway/compare/v2.15.1...v2.15.2)

#### What's Changed

- Switch the use of glog to grpclog. by
[@&#8203;jmuk](https://togithub.com/jmuk) in
[grpc-ecosystem/grpc-gateway#3205
- fix: remove `push_bsr_plugins` Github Action job by
[@&#8203;gunturaf](https://togithub.com/gunturaf) in
[grpc-ecosystem/grpc-gateway#3203
- Add correct comment, remove redundant vars and error, use short form
by [@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3206

#### New Contributors

- [@&#8203;jmuk](https://togithub.com/jmuk) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3205
- [@&#8203;gunturaf](https://togithub.com/gunturaf) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3203

**Full Changelog**:
grpc-ecosystem/grpc-gateway@v2.15.1...v2.15.2

###
[`v2.15.1`](https://togithub.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.15.1)

[Compare
Source](https://togithub.com/grpc-ecosystem/grpc-gateway/compare/v2.15.0...v2.15.1)

#### What's Changed

- Clean up use_allof_for_refs flag option by
[@&#8203;warrenb95](https://togithub.com/warrenb95) in
[grpc-ecosystem/grpc-gateway#3092
-
fix([#&#8203;3049](https://togithub.com/grpc-ecosystem/grpc-gateway/issues/3049)):add
items type for array by [@&#8203;li31727](https://togithub.com/li31727)
in
[grpc-ecosystem/grpc-gateway#3090
- fix: JSON examples in YAML output
\[[#&#8203;3095](https://togithub.com/grpc-ecosystem/grpc-gateway/issues/3095)]
by [@&#8203;hedhyw](https://togithub.com/hedhyw) in
[grpc-ecosystem/grpc-gateway#3106
- feat(3102):update DefaultHeaderMatcher function comment docs by
[@&#8203;li31727](https://togithub.com/li31727) in
[grpc-ecosystem/grpc-gateway#3114
- Rename root BUILD to BUILD.bazel for consistency by
[@&#8203;rsepassi](https://togithub.com/rsepassi) in
[grpc-ecosystem/grpc-gateway#3117
- Add `format` in path params by
[@&#8203;antgamdia](https://togithub.com/antgamdia) in
[grpc-ecosystem/grpc-gateway#3089
- Change verb by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3129
- Remove redundant err and use %w verb for err by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3130
- Use ifshort stmt for err by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3131
- Cleanup by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3137
- Change '%v' with %q verb and small cleanup by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3139
- fix: Partial resource update, no longer generate updateMask field by
[@&#8203;csh995426531](https://togithub.com/csh995426531) in
[grpc-ecosystem/grpc-gateway#3093
- Add go1.20 to ci matrix by
[@&#8203;sashamelentyev](https://togithub.com/sashamelentyev) in
[grpc-ecosystem/grpc-gateway#3176
- Update README.md by
[@&#8203;paulburlumi](https://togithub.com/paulburlumi) in
[grpc-ecosystem/grpc-gateway#3184
- gateway: Dial -> DialContext by
[@&#8203;torkelrogstad](https://togithub.com/torkelrogstad) in
[grpc-ecosystem/grpc-gateway#3190
- Buf version bump by
[@&#8203;torkelrogstad](https://togithub.com/torkelrogstad) in
[grpc-ecosystem/grpc-gateway#3189
- Fix bug with non-ASCII HTTP headers by
[@&#8203;steinarvk-oda](https://togithub.com/steinarvk-oda) in
[grpc-ecosystem/grpc-gateway#3197

#### New Contributors

- [@&#8203;warrenb95](https://togithub.com/warrenb95) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3092
- [@&#8203;li31727](https://togithub.com/li31727) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3090
- [@&#8203;rsepassi](https://togithub.com/rsepassi) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3117
- [@&#8203;antgamdia](https://togithub.com/antgamdia) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3089
- [@&#8203;csh995426531](https://togithub.com/csh995426531) made their
first contribution in
[grpc-ecosystem/grpc-gateway#3093
- [@&#8203;paulburlumi](https://togithub.com/paulburlumi) made their
first contribution in
[grpc-ecosystem/grpc-gateway#3184
- [@&#8203;steinarvk-oda](https://togithub.com/steinarvk-oda) made their
first contribution in
[grpc-ecosystem/grpc-gateway#3197

**Full Changelog**:
grpc-ecosystem/grpc-gateway@v2.15.0...v2.15.1

</details>

<details>
<summary>grpc/grpc-go</summary>

### [`v1.3.0`](https://togithub.com/grpc/grpc-go/releases/tag/v1.3.0):
Release 1.3.0

[Compare
Source](https://togithub.com/grpc/grpc-go/compare/v1.2.0...v1.3.0)

### API change

- Never encode binary metadata within the metadata map
([#&#8203;1188](https://togithub.com/grpc/grpc-go/issues/1188))
- Update grpclb proto and move grpclb into package grpc
([#&#8203;1186](https://togithub.com/grpc/grpc-go/issues/1186))
- Change status package to deal with concrete types instead of
interfaces
([#&#8203;1171](https://togithub.com/grpc/grpc-go/issues/1171))
- Behavior change: do not strip out gRPC user-agent
([#&#8203;1158](https://togithub.com/grpc/grpc-go/issues/1158))
- Separate incoming and outgoing metadata in context
([#&#8203;1157](https://togithub.com/grpc/grpc-go/issues/1157))
- Add status package for reporting gRPC status and errors
([#&#8203;1156](https://togithub.com/grpc/grpc-go/issues/1156))
- remove support for go1.5
([#&#8203;1132](https://togithub.com/grpc/grpc-go/issues/1132))

### New Feature

- Client load report for grpclb.
([#&#8203;1200](https://togithub.com/grpc/grpc-go/issues/1200))
- Client should update keepalive parameters upon receiving GoAway
([#&#8203;1169](https://togithub.com/grpc/grpc-go/issues/1169))
- Implementation for server enforcement of keepalive policy.
([#&#8203;1147](https://togithub.com/grpc/grpc-go/issues/1147))
- Add grpc.Version string and use it in the UA
([#&#8203;1144](https://togithub.com/grpc/grpc-go/issues/1144))
- Support max
age([#&#8203;1119](https://togithub.com/grpc/grpc-go/issues/1119))
- Support proxy with dialer
([#&#8203;1098](https://togithub.com/grpc/grpc-go/issues/1098))

### Behavior change

- populate initReq target name and fix IP \[]byte type in grpclb
([#&#8203;1145](https://togithub.com/grpc/grpc-go/issues/1145))
- pick a random address if the current in use is deleted by resolver
([#&#8203;1135](https://togithub.com/grpc/grpc-go/issues/1135))
- :authority should include port number
([#&#8203;1123](https://togithub.com/grpc/grpc-go/issues/1123))
- Don't return an error from dial if the balancer returns no initial
servers ([#&#8203;1112](https://togithub.com/grpc/grpc-go/issues/1112))

### Bug fix

- Fix nil pointer dereferences from status.FromProto(nil)
([#&#8203;1211](https://togithub.com/grpc/grpc-go/issues/1211))
- Use unpadded base64 encoding for binary metadata headers; handle
padded or unpadded input
([#&#8203;1209](https://togithub.com/grpc/grpc-go/issues/1209))
- Use proto.Equal for equalities on Go proto messages
([#&#8203;1204](https://togithub.com/grpc/grpc-go/issues/1204))
- Move handling stats.End to clientStream.finish()
([#&#8203;1182](https://togithub.com/grpc/grpc-go/issues/1182))
- grpclb should connect to the second balancer
([#&#8203;1181](https://togithub.com/grpc/grpc-go/issues/1181))
- add error handling for InvalidArgument error from sendResponse()
([#&#8203;1173](https://togithub.com/grpc/grpc-go/issues/1173))
- transport: implement GoString on Stream
([#&#8203;1167](https://togithub.com/grpc/grpc-go/issues/1167))
- Bug
fix([Issue#&#8203;1141](https://togithub.com/Issue/grpc-go/issues/1141)):
Check if peer is nil before trying to derefer it.
([#&#8203;1143](https://togithub.com/grpc/grpc-go/issues/1143))
- Make sure all in-flight streams close when ClientConn.Close() is
called. ([#&#8203;1136](https://togithub.com/grpc/grpc-go/issues/1136))

### Performance

- opt in to frame reuse on the framer to reduce garbage
([#&#8203;1096](https://togithub.com/grpc/grpc-go/issues/1096))
- use proto.Buffer API for protobuf codec and cache proto.Buffer structs
([#&#8203;1010](https://togithub.com/grpc/grpc-go/issues/1010))

### Documentation

- add document to ClientHandshake about returning temporary error
([#&#8203;1125](https://togithub.com/grpc/grpc-go/issues/1125))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on wednesday" in timezone
Australia/Sydney, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/google/osv.dev).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNDYuMiIsInVwZGF0ZWRJblZlciI6IjM0LjE0Ni4yIn0=-->
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.

Partial resource update RPCs include the update mask in the swagger specification
2 participants