Skip to content
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

Question, maybe add to documentation request: HTTP request to CloudEvent #766

Closed
salaboy opened this issue Apr 1, 2022 · 8 comments · Fixed by #799
Closed

Question, maybe add to documentation request: HTTP request to CloudEvent #766

salaboy opened this issue Apr 1, 2022 · 8 comments · Fixed by #799

Comments

@salaboy
Copy link

salaboy commented Apr 1, 2022

Team, a bit of a newbie question, but I couldn't find the answer in the docs and maybe other people will have the same issue.
Is there any simple way to transform an incoming HTTP request to a CloudEvent and fail with an error if the request doesn't contain the correct headers to be a CloudEvent?

I am looking for something along the lines of cloudevents.NewEvent(req), but I understand that this should come from the HTTP binding.
I've found protocol/http.NewMessageFromHttpRequest(req) but again this seems to be transforming to an internal representation that is still one step away from being a CloudEvent.

If there is one, it will be great to add that into the docs.. as most of the docs show how to create Handler functions that will automatically parse the HTTP request into a CloudEvent, but not the inner workings.

If this is not recommended for some reason, it will be also good to add that to the docs.

@dan-j
Copy link
Contributor

dan-j commented Apr 1, 2022

Feel free to create a PR with some documentation where you'd expect to see it within the docs/ directory. I believe you could do something like this:

import (
    "github.com/cloudevents/sdk-go/v2/binding"
    cehttp "github.com/cloudevents/sdk-go/v2/protocol/http"
)

...

var req *http.Request
msg := cehttp.NewMessageFromHttpRequest(req)
event, err := binding.ToEvent(context.Background(), msg, nil)
if err != nil {
    panic(err)
}

fmt.Println(event.ID())

@salaboy
Copy link
Author

salaboy commented Apr 2, 2022

@dan-j thanks for sharing that example, that is really helpful. But I wonder what is your opinion on making that simple and more intuitive for users, meaning something like:

event, err := cehttp.NewEventFromHttpRequest(req); 

This is just encapsulating the code that you shared...

I am happy to send a PR for this, if the team behind the SDK think this is a good or OK idea :)

Cheers

@n3wscott
Copy link
Member

n3wscott commented Apr 6, 2022

What to add a PR for this utility method?

@grayside
Copy link
Contributor

Where should this method live?

@n3wscott
Copy link
Member

n3wscott commented Sep 1, 2022

I think a new file in https://github.com/cloudevents/sdk-go/tree/main/v2/protocol/http would be fine, and you could export it to the top level via https://github.com/cloudevents/sdk-go/blob/main/v2/alias.go

@grayside
Copy link
Contributor

grayside commented Sep 1, 2022

I've got most of a first pass put together, but I'm getting a segfault that I've traced to the transformers invocation in the ToEvent return statement:

return &e, Transformers(transformers).Transform((*EventMessage)(&e), encoder)

This struck me as potentially an existing bug, so I wanted to surface this quickly in case it was already identified.

@n3wscott
Copy link
Member

n3wscott commented Sep 6, 2022

interesting, any idea what the segfault is about? able to make a unit test to repro?

@grayside
Copy link
Contributor

grayside commented Sep 6, 2022

Yes, the unit test coverage I wrote for the helper function consistently reproduces. I've pushed what I have to a draft PR to illustrate: #799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants