-
Notifications
You must be signed in to change notification settings - Fork 224
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
Add sender example for protobuf data #712
Conversation
This example mirrors the existing HTTP sender example except that it adds a protobuf object as the payload instead of a JSON map. Signed-off-by: Kevin Conway <kevinconway@invisionapp.com>
2e9fa49
to
ba2040d
Compare
LGTM, I've never contributed to the http aspects but since this is just adding a sample I guess there's nothing really to be worried about. @n3wscott? |
e.SetType("com.cloudevents.sample.sent") | ||
e.SetSource("https://github.com/cloudevents/sdk-go/v2/samples/http/sender-protobuf") | ||
e.SetDataSchema(string(data.ProtoReflect().Descriptor().FullName())) | ||
_ = e.SetData(pbcloudevents.ContentTypeProtobuf, data) |
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.
oh! only the data is encoded in protobuf? I had assumed the entire cloudevent was being sent as a protobuf
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.
Haven't looked into the code, but maybe line 22 tells the encoder to use protobuf?
LGTM, Can you clarify if it is possible to use proto for the entire message, or just the data? Thanks for adding a sample!! |
Signed-off-by: Kevin Conway <kevinconway@invisionapp.com>
1ccb62a
to
a03b247
Compare
Signed-off-by: Kevin Conway <kevinconway@invisionapp.com>
cd636dd
to
3830cf2
Compare
@n3wscott @dan-j I added a protobuf receiver that can be run together with the sender for a full end-to-end sample. Unfortunately, this only demonstrates the data encoding support and does not demonstrate the envelope format. The issue is that HTTP, and the other protocols, already implement a custom envelope format in the form of headers. For example, the protobuf sender makes requests like:
The content body is encoded with protobuf but the envelop is conveyed via the headers. This seems true for all protocols implemented for Go except nats (which requires JSON format according to the spec) and websockets (which has no server support with which I could create the demo). This makes it difficult to demo an end-to-end usage of the protobuf envelope format. I'm open to suggestions if folks can suggest how I might create the format sample. For my usage of protobuf+CloudEvents, we've defined a protobuf service that accepts the CloudEvent structure. Example: service SinkService {
rpc Send(CloudEvent) returns (Empty);
rpc Request(CloudEvent) returns (CloudEvent);
} This seems like a potentially missing part of the protobuf spec that could then be used to generate gRCP (or other) implementations of the service/client wrapped in the sender and receiver interfaces. I think something like that would provide a path for a protobuf native suite of protocols. I also addressed an issue raised by a comment I can't seem to find anymore about the data schema failing validation. I was including the type of the protobuf message as the data schema but the CloudEvent spec requires the field to be an absolute URI. I've adjusted the example to use an absolute URI and ran the sample to ensure it sends requests as expected. |
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.
LGTM
This example mirrors the existing HTTP sender example except that it
adds a protobuf object as the payload instead of a JSON map.