Skip to content

status: Fix status incompatibility introduced by #6919 and move non-regeneratable proto code into /testdata #7724

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 22, 2024
Merged

status: Fix status incompatibility introduced by #6919 and move non-regeneratable proto code into /testdata #7724

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
73e33f2
New directory for status proto
arjan-bal Oct 10, 2024
31d53ae
Move non-regeratable files into a common testdata directory
arjan-bal Oct 10, 2024
3b088a0
Add note about only v2 API
arjan-bal Oct 10, 2024
a0c3b84
Add a new proto to the test case
arjan-bal Oct 10, 2024
25cf51e
address review comments
arjan-bal Oct 17, 2024
8633ca3
fix test case
arjan-bal Oct 18, 2024
7dd0e15
rename directory
arjan-bal Oct 21, 2024
6009694
Merge branch 'master' of github.com:grpc/grpc-go into fix-status-inco…
arjan-bal Oct 21, 2024
ecdbda8
Merge remote-tracking branch 'source/master' into fix-status-incompat…
arjan-bal Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78
github.com/envoyproxy/go-control-plane v0.13.0
github.com/golang/glog v1.2.2
github.com/golang/protobuf v1.5.4
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
golang.org/x/net v0.29.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ github.com/envoyproxy/protoc-gen-validate v1.1.0 h1:tntQDh69XqOCOZsDz0lVJQez/2L6
github.com/envoyproxy/protoc-gen-validate v1.1.0/go.mod h1:sXRDRVmzEbkM7CVcM06s9shE/m23dg3wzjl0UWqJ2q4=
github.com/golang/glog v1.2.2 h1:1+mZ9upx1Dh6FmUTFR1naJ77miKiXgALjWOZ3NVFPmY=
github.com/golang/glog v1.2.2/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
Expand Down
35 changes: 34 additions & 1 deletion internal/status/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ func (s *Status) WithDetails(details ...protoadapt.MessageV1) (*Status, error) {

// Details returns a slice of details messages attached to the status.
// If a detail cannot be decoded, the error is returned in place of the detail.
// If the detail can be decoded, the proto message returned is of the same
// type that was given to WithDetails().
func (s *Status) Details() []any {
if s == nil || s.s == nil {
return nil
Expand All @@ -160,7 +162,38 @@ func (s *Status) Details() []any {
details = append(details, err)
continue
}
details = append(details, detail)
// The call to MessageV1Of is required to unwrap the proto message if
// it implemented only the MessageV1 API. The proto message would have
// been wrapped in a V2 wrapper in Status.WithDetails. V2 messages are
// added to a global registry used by any.UnmarshalNew().
// MessageV1Of has the following behaviour:
// 1. If the given message is a wrapped MessageV1, it returns the
// unwrapped value.
// 2. If the given message already implements MessageV1, it returns it
// as is.
// 3. Else, it wraps the MessageV2 in a MessageV1 wrapper.
//
// Since the Status.WithDetails() API only accepts MessageV1, calling
// MessageV1Of ensures we return the same type that was given to
// WithDetails:
// * If the give type implemented only MessageV1, the unwrapping from
// point 1 above will restore the type.
// * If the given type implemented both MessageV1 and MessageV2, point 2
// above will ensure no wrapping is performed.
// * If the given type implemented only MessageV2 and was wrapped using
// MessageV1Of before passing to WithDetails(), it would be unwrapped
// in WithDetails by calling MessageV2Of(). Point 3 above will ensure
// that the type is wrapped in a MessageV1 wrapper again before
// returning. Note that protoc-gen-go doesn't generate code which
// implements ONLY MessageV2 at the time of writing.
//
// NOTE: Status details can also be added using the FromProto method.
// This could theoretically allow passing a Detail message that only
// implements the V2 API. In such a case the message will be wrapped in
// a MessageV1 wrapper when fetched using Details().
// Since protoc-gen-go generates only code that implements both V1 and
// V2 APIs for backward compatibility, this is not a concern.
Comment on lines +194 to +195
Copy link
Member

Choose a reason for hiding this comment

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

Is this guaranteed to always be the case? (Does it matter?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this guaranteed to always be the case?

I don't think so. The V1 API is deprecated and is only present for backward compatibility. There is no guarantee that protoc-gen-go will always support it.

Does it matter?

protoadapt.MessageV1Of will return a wrapped message which implements the V1 API when the argument doesn't implement it already. Callers will need to unwrap it using the protoadapt.MessageV2Of to get the inner type.

This is why I updated the doc comment to say the following:

If the detail can be decoded, the proto message returned is of the same type that was given to WithDetails().

status.WithDetails accepts only V1 messages. To pass it a V2 message, it will need to be wrapped and status.Details() will return the same wrapped type.

details = append(details, protoadapt.MessageV1Of(detail))
}
return details
}
Expand Down
2 changes: 2 additions & 0 deletions interop/xds/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
Expand Down
2 changes: 1 addition & 1 deletion reflection/test/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ go 1.22.7
replace google.golang.org/grpc => ../../

require (
github.com/golang/protobuf v1.5.4
Copy link
Member

Choose a reason for hiding this comment

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

IIRC, the only reason this file was here is to hide this module dependency, since it's deprecated. Let's just delete this file now. I think we're going to have to accept a huge module dependency list, which is why I created #7690 - to make sure our actual production code dependencies can be tracked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the go.mod and go.sum files from reflection/test/.

google.golang.org/grpc v1.66.2
google.golang.org/protobuf v1.34.2
)

require (
github.com/golang/protobuf v1.5.4 // indirect
golang.org/x/net v0.29.0 // indirect
golang.org/x/sys v0.25.0 // indirect
golang.org/x/text v0.18.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion reflection/test/serverreflection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import (
v1reflectionpb "google.golang.org/grpc/reflection/grpc_reflection_v1"
v1alphareflectiongrpc "google.golang.org/grpc/reflection/grpc_reflection_v1alpha"
pb "google.golang.org/grpc/reflection/grpc_testing"
pbv3 "google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate"
pbv3 "google.golang.org/grpc/testdata/grpc_testing_not_regenerate"
)

var (
Expand Down
2 changes: 1 addition & 1 deletion scripts/regenerate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,6 @@ mv "${WORKDIR}"/out/google.golang.org/grpc/lookup/grpc_lookup_v1/* "${WORKDIR}/o

# grpc_testing_not_regenerate/*.pb.go are not re-generated,
# see grpc_testing_not_regenerate/README.md for details.
rm "${WORKDIR}"/out/google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate/*.pb.go
rm "${WORKDIR}"/out/google.golang.org/grpc/testdata/grpc_testing_not_regenerate/*.pb.go

cp -R "${WORKDIR}"/out/google.golang.org/grpc/* .
4 changes: 2 additions & 2 deletions scripts/vet.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ git grep -l 'interface{}' -- "*.go" 2>&1 | not grep -v '\.pb\.go\|protoc-gen-go-
git grep -l -e 'grpclog.I' --or -e 'grpclog.W' --or -e 'grpclog.E' --or -e 'grpclog.F' --or -e 'grpclog.V' -- "*.go" | not grep -v '^grpclog/component.go\|^internal/grpctest/tlogger_test.go\|^internal/grpclog/prefix_logger.go'

# - Ensure that the deprecated protobuf dependency is not used.
not git grep "\"github.com/golang/protobuf/*" -- "*.go" ':(exclude)reflection/test/grpc_testing_not_regenerate/*'
not git grep "\"github.com/golang/protobuf/*" -- "*.go" ':(exclude)testdata/grpc_testing_not_regenerate/*'

# - Ensure all usages of grpc_testing package are renamed when importing.
not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
Expand Down Expand Up @@ -188,7 +188,7 @@ done
# Error for violation of enabled lint rules in config excluding generated code.
revive \
-set_exit_status=1 \
-exclude "reflection/test/grpc_testing_not_regenerate/" \
-exclude "testdata/grpc_testing_not_regenerate/" \
-exclude "**/*.pb.go" \
-formatter plain \
-config "$(dirname "$0")/revive.toml" \
Expand Down
2 changes: 2 additions & 0 deletions security/advancedtls/examples/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
Expand Down
2 changes: 2 additions & 0 deletions security/advancedtls/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
Expand Down
2 changes: 2 additions & 0 deletions stats/opencensus/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,8 @@ github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaS
github.com/golang/protobuf v1.5.1/go.mod h1:DopwsBzvsk0Fs44TXzsVbJyPhcCPeIwnvohx4u74HPM=
github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
Expand Down
2 changes: 2 additions & 0 deletions stats/opentelemetry/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
Expand Down
31 changes: 31 additions & 0 deletions status/status_ext_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ import (
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/protoadapt"
"google.golang.org/protobuf/testing/protocmp"

testpb "google.golang.org/grpc/interop/grpc_testing"
tpb "google.golang.org/grpc/testdata/grpc_testing_not_regenerate"
)

const defaultTestTimeout = 10 * time.Second
Expand Down Expand Up @@ -203,3 +205,32 @@ func (s) TestStatusDetails(t *testing.T) {
})
}
}

// TestStatus_ErrorDetailsMessageV1 verifies backward compatibility of the
// status.Details() method when using protobuf code generated with only the
// MessageV1 API implementation.
func (s) TestStatus_ErrorDetailsMessageV1(t *testing.T) {
details := []protoadapt.MessageV1{
&tpb.SimpleMessage{
Data: "abc",
},
&testpb.Empty{},
Copy link

Choose a reason for hiding this comment

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

According to the test comments, should testpb.Empty be removed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, removed. I wanted to keep a test case for ensuring v1 + v2 messages are also handled correctly, added a new test function for that later.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit (optional)

I feel this is more readable and compact:

	details := []protoadapt.MessageV1{
		&tpb.SimpleMessage{Data: "abc"},
		&testpb.Empty{},
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested.

s, err := status.New(codes.Aborted, "").WithDetails(details...)
if err != nil {
t.Fatalf("(%v).WithDetails(%+v) failed: %v", s, details, err)
}
got := s.Details()
gotDetails := []protoadapt.MessageV1{}
for i := range got {
msg, ok := got[i].(protoadapt.MessageV1)
if !ok {
t.Fatalf("(%v).Details() returned message that doesn't implement protoadapt.MessageV1: %v", s, got[i])
}
gotDetails = append(gotDetails, msg)
}

if diff := cmp.Diff(details, gotDetails, protocmp.Transform()); diff != "" {
Copy link

Choose a reason for hiding this comment

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

I just have glance on cmp.Diff, look like it is used to convert proto.Message into map-like structure, then perform a recursive "value-equal" check.
I think such examination does not satisfy "the proto message returned by Details() is of the same type that was given to WithDetails()". A reflect.Type check is preferred in such situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, added a type comparison using reflect.TypeOf.

t.Errorf("(%v).Details got unexpected output, diff (-got +want):\n%s", s, diff)
Copy link
Contributor

Choose a reason for hiding this comment

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

The order in the output, (-got +want), needs to be swapped I guess based on the order of parameters passed to cmp.Diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}
}
4 changes: 4 additions & 0 deletions ...est/grpc_testing_not_regenerate/README.md → ...ata/grpc_testing_not_regenerate/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ with `"context"`.

`dynamic.go` was generated with a newer protoc and manually edited to remove
everything except the descriptor bytes var, which is renamed and exported.

`simple_message_v1.go` was generated using protoc-gen-go v1.3.5 which doesn't
support the MesssageV2 API. As a result the generated code implements only the
old MessageV1 API.
0 ...st/grpc_testing_not_regenerate/dynamic.go → ...ta/grpc_testing_not_regenerate/dynamic.go
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions ...grpc_testing_not_regenerate/dynamic.proto → ...grpc_testing_not_regenerate/dynamic.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@

syntax = "proto3";

option go_package = "google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate";

package grpc.testing;

option go_package = "google.golang.org/grpc/testdata/grpc_testing_not_regenerate";

message DynamicRes {}

message DynamicReq {}
Expand Down
27 changes: 27 additions & 0 deletions testdata/grpc_testing_not_regenerate/simple.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2022 gRPC authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be 2024?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

syntax = "proto3";

package grpc.testdata.grpc_testing_not_regenerate;

option go_package = "google.golang.org/grpc/testdata/grpc_testing_not_regenerate";

// SimpleMessage is used to hold string data.
message SimpleMessage {
Copy link

Choose a reason for hiding this comment

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

I feel it is better to name the test message as MessageV1Only, and give a short comments describing why it must not be regenerated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like "implementing the V1 API" is a property of the generated code and not the proto itself. There is a README.md describing the use of each generated file and how it was generated.

Copy link

Choose a reason for hiding this comment

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

Sorry I missed that.

string data = 1;
}
83 changes: 83 additions & 0 deletions testdata/grpc_testing_not_regenerate/simple_message_v1.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 ...est/grpc_testing_not_regenerate/testv3.go → ...ata/grpc_testing_not_regenerate/testv3.go
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions .../grpc_testing_not_regenerate/testv3.proto → .../grpc_testing_not_regenerate/testv3.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@

syntax = "proto3";

option go_package = "google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate";

package grpc.testingv3;

option go_package = "google.golang.org/grpc/testdata/grpc_testing_not_regenerate";

message SearchResponseV3 {
message Result {
string url = 1;
Expand Down
Loading