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

Adjust response handling for CloudEvents route #904

Open
thjaeckle opened this issue Dec 1, 2020 · 2 comments
Open

Adjust response handling for CloudEvents route #904

thjaeckle opened this issue Dec 1, 2020 · 2 comments

Comments

@thjaeckle
Copy link
Member

thjaeckle commented Dec 1, 2020

Currently with #889 and #895 the /cloudevents POST endpoint acts as "sink" only:

  • accepting (HTTP code 202) all successfully persisted Ditto Protocol commands
  • for errors (e.g. missing permissions, conflict, other "bad requests), responding with the corresponding HTTP code and the error payload

However, no response payload in "Ditto Protocol JSON" is returned.
The cloud event spec for HTTP defines:

The event binding is identical for both HTTP request and response messages.

IMO the route's response behavior has to be adjusted:

  • the content-mode must be preserved (when invocation did happen with "structured content mode", the response must be also in "structured content mode")
  • the passed-in cloudevent headers have to be preserved and returned in the HTTP response
  • the response payload must also be in Ditto Protocol JSON
    • the response payload could also be defined by specifying the Accept HTTP header and applying content negotiation
@ctron
Copy link
Contributor

ctron commented Dec 4, 2020

To my understanding the following:

The event binding is identical for both HTTP request and response messages.

Simply states that the cloud event binding for HTTP can be used the same way for request and response. However there is no requirement that the endpoint must response with a cloud event.

Taking a look at the example code from cloudevents seems to confirm this:

https://github.com/cloudevents/sdk-rust/blob/6f5a767f196cea6bf732cde9680c1c233fbddf80/example-projects/actix-web-example/src/main.rs#L6-L11

https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/samples/http-json-cloudevents/json_sample_server.py#L33

However, I think it may make sense for Ditto to respond with something other than "ok, I processed the event". If Ditto can provide additional information, as the outcome of the operation, then it could provide this.

I am not sure how the Accept header fits into the picture. I don't think the header is used by the cloud events spec. And so I don't think Ditto should introduce a new concept, which is not covered by the HTTP cloud events mapping.

@yufei-cai
Copy link
Contributor

The cloudevents endpoint cannot support query, message and search commands without responses. I'm fine with not supporting those, especially search commands where additional session management would be necessary, but the error message should probably be clearer than "Query commands must not have the header 'response-required' set to 'false'".

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

No branches or pull requests

3 participants