-
Notifications
You must be signed in to change notification settings - Fork 89
chore: update golang.google.org/grpc dep from v1.71.1 to v1.72.0 #777
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
Changes from all commits
e4b6302
1194d51
ab01acf
8c322d2
4e979f7
c4b8a53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
bin/* | ||
Dockerfile.cross | ||
artifacts | ||
main | ||
|
||
# Test binary, built with `go test -c` | ||
*.test | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,12 +53,12 @@ spec: | |
livenessProbe: | ||
grpc: | ||
port: 9003 | ||
service: inference-extension | ||
service: envoy.service.ext_proc.v3.ExternalProcessor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does this represent, would it matter if we remove it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this should match the name in the health check routine, otherwise we return Wondering why we want to take a dependency on what the service name as defined in https://github.com/envoyproxy/go-control-plane/blob/main/envoy/service/ext_proc/v3/external_processor_grpc.pb.go#L135 when we could control that on our end? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To my understanding the gRPC service might not be able to change because ext_proc expects this specific one by name to be present for its requests. At least by default, maybe that's configurable somewhere (on the Envoy side)? In any case, this is the actual service name we've been using since the beginning: if you turn on reflection on the current gRPC server and do a list, this is the exposed service that Envoy is expecting. |
||
initialDelaySeconds: 5 | ||
periodSeconds: 10 | ||
readinessProbe: | ||
grpc: | ||
port: 9003 | ||
service: inference-extension | ||
service: envoy.service.ext_proc.v3.ExternalProcessor | ||
initialDelaySeconds: 5 | ||
periodSeconds: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
musing: Wondering if this will bite someone later. I dont expect us to have a
main
dir or file at root. Maybe this is preferredUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the default binary a
go build
emits, so it was just buggin me because I was building the binaries locally and didn't want it to try and check that in. Wasn't really thinking anyone would actually add this at any point, but if you strongly prefer we don't add this though, no sweat I'll remove it.