-
-
Notifications
You must be signed in to change notification settings - Fork 75
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: import public causes AssertionError #82
Comments
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes hyperledger-cacti#1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes hyperledger-cacti#1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Hey. Thank you for reporting this. I will try to take a look this week. |
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes hyperledger-cacti#1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes #1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Update; Upon closer look, I realized that TLDR: It is not so easy to solve this problem of re-exporting transitive dependencies. // old.proto
syntax = "proto3";
import public "public.proto";
import public "public2.proto";
message OldMarker {
importdirective.PublicMessage pubMessageField = 4;
importdirective.PublicMessage2 pubMessageField2 = 5;
} // public.proto
syntax = "proto3";
package importdirective;
message PublicMessage {
bool pb = 1;
} // public2.proto
syntax = "proto3";
package importdirective;
message PublicMessage2 {
bool pb = 1;
} this will lead to export conflicts. in order to work around this, we need to explicitly import symbols from transitive dependencies and re-export the symbols from them within a namespace whose name is identical to conflicting this is what the protoc compiler wants us to do. merge everything under a namespace if the protos has the same package directive hence it is safe to do because protoc guarantees that there can not be identically named symbols under the same name |
But still, there are more cursed cases; syntax = "proto3";
package importdirective;
import public "test/import/public.proto";
import public "test/import/public2.proto";
message OldMarker {
importdirective.PublicMessage pubMessageField = 4;
importdirective.PublicMessage2 pubMessageField2 = 5;
} where we need to re-export everything that conflicts as well as the symbols in the proto within the same package. |
@thesayyn Quite the can of worms indeed. :-) Thank you again for the investigation/fix efforts! |
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes hyperledger-cacti#1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Also seen as: protocolbuffers/protobuf-javascript#55 in js version. This is something that we definitely have to solve here. |
Primary change: -------------- The API server now contains a gRPC server in addition to what it had before (HTTP+SocketIO servers) so that through this it can provide the opportunity for plugins to expose their gRPC services via the Cactus API server. It is possible for plugins to serve their requests on multiple protocols at the same time which means that this is a big step towards our goal of being properly multi- protocol capable. Secondary change(s): ------------------- 1. Custom protobuf-schema openapi generator template added because of this issue: thesayyn/protoc-gen-ts#82 (we override the stock template to not use the public keyword) TODO: ---- 1. Implement streaming healthcheck endpiont with gRPC 2. Allow plugins to hook in their own gRPC service implementations. Fixes hyperledger-cacti#1189 Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
I am going to close this as not planned. |
@thesayyn Cool, thanks for the update! Hopefully the upstream repo adds support to it at some point. |
This works fine:
This one crashes protoc:
Logs:
The text was updated successfully, but these errors were encountered: