-
Notifications
You must be signed in to change notification settings - Fork 217
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 EventResponse for send/receive. #42
Conversation
…alue on recieve. Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Note: This is dependent on #45 |
Signed-off-by: Scott Nichols <nicholss@google.com>
…the defaulter chain to the outbound message. Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
HTTP Receive Sample now outputs like:
|
fmt.Printf("Got Event Context: %+v\n", event.Context) | ||
data := &Example{} | ||
if err := event.DataAs(data); err != nil { | ||
fmt.Printf("Got Data Error: %s\n", err.Error()) | ||
} | ||
fmt.Printf("Got Data: %+v\n", data) | ||
|
||
fmt.Printf("Got Transport Context: %+v\n", http.TransportContextFrom(ctx)) |
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.
look at this @Harwayne ^^
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.
That seems to include the HTTP host, path, and method. Nifty.
cehttp.WithPort(env.Port), | ||
cehttp.WithPath(env.Path), | ||
client.WithUUIDs(), | ||
client.WithTimeNow(), |
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 REALLY subtle and awesome. I am setting client options on the http client wrapper, and using those event defaulters to default values on the response event. 💯
// Receive is called from from the transport on event delivery. | ||
func (c *ceClient) Receive(ctx context.Context, event cloudevents.Event, resp *cloudevents.EventResponse) error { | ||
if c.receive != nil { | ||
err := c.receive(ctx, event, resp) |
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.
client.receive should be mappable to an event type or other interesting filtering. Look into this.
*resp.Event = fn(*resp.Event) | ||
} | ||
// Validate the event conforms to the CloudEvents Spec. | ||
if err := resp.Event.Validate(); err != nil { |
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.
AND!!! outbound events are validated.
} | ||
defer resp.Body.Close() | ||
|
||
body, _ := ioutil.ReadAll(resp.Body) |
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 gathering the response and smashing it into an event if it can.
This PR enables control over response codes for a transport (transport dependent). For HTTP this means letting the receiver control which HTTP status code is returned, as well as a response cloudevents.Event object. That response object will be passed through the client's defaulter chain if it the client is used to Send/Receive events ✨.
The signature of client.Receive has changed significantly, but in a followup PR I will reduce its complexity once again, and the README will not be wrong with the following:
Until then, I want to leave the README in a broken state, because the samples do work and after the next PR, the README will be correct as well.
Fixes: #31
Signed-off-by: Scott Nichols nicholss@google.com