s2a: Change S2A proto package name #164
Conversation
|
|
||
| option java_multiple_files = true; | ||
| option java_outer_classname = "CommonProto"; | ||
| option java_package = "io.grpc.s2a.internal.handshaker"; |
There was a problem hiding this comment.
This conflicts with the internal proto. I think we need to have a better undestanding of the plan.
There was a problem hiding this comment.
Thanks for pointing this out Eric. From our email thread about this proto package name change, and what you pointed out about conflicting java package names, my understanding is there are a few steps:
- introduce a new proto here, with the package name
s2a.proto.v2, with a java package name ofio.grpc.s2a.internal.proto(or something else). - once this is merged, delete the protos in gRPC-Java and replace them with these protos (with a directory structure: s2a/src/main/proto/grpc/gcp/s2a/proto/v2/). This will require importing the new protos from package
io.grpc.s2a.internal.protoin implementation files (currently the implementation can use protos without importing them because they are in the same java package) - delete the old protos in this repo
- delete the old protos in gRPC-Java
It would probably be preferable to have the protos continue to live in the same java package as the implementation (@matthewstevenson88, please let me know if it doesn't matter) , in this case, I think we would need to take a different sequence of steps:
- update protos in gRPC-Java to have the package name
s2a.proto.v2 - send out this PR which creates the new protos with package name
s2a.proto.v2 - in a followup PR delete the old protos in this package
However I feel like the second process I mentioned might not be feasible, since IIUC, new protos need to be added in this repo before they can be updated in gRPC-Java.
WDYT? If the first sequence is ok with you and @matthewstevenson88 , we can go ahead and implement it.
There was a problem hiding this comment.
I'm talking about something completely different here. You can't have two copies of the same proto with different java packages without strong caution. This proto already exists internally and doesn't look like this. We need to talk about what version will win and how that will be done.
It's also against conventions to have a proto named s2a.proto and then use the Java package io.grpc.s2a. You'd normally use com.s2a or the like. We can do some funny business here, but the entire world uses a single name. And thus needing to figure out the relationship with the protos inside Google.
|
|
||
| syntax = "proto3"; | ||
|
|
||
| package s2a.proto.v2; |
There was a problem hiding this comment.
The folder structure needs to match the package name, so this would live in the top-level folder s2a.
After this gets merged, we can modify grpc-java s2a to use the new protos, and then delete the old copy at gcp/s2a/common.proto, gcp/s2a/s2a.proto, gcp/s2a/s2a_context.proto