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

Webhook subscription mechanism #325

Merged
merged 5 commits into from
Dec 7, 2022
Merged

Webhook subscription mechanism #325

merged 5 commits into from
Dec 7, 2022

Conversation

mlagally
Copy link
Contributor

@mlagally mlagally commented Nov 15, 2022

@mlagally
Copy link
Contributor Author

mlagally commented Nov 23, 2022

Arch call on Nov 23rd:

  • Align URL with URI wrt. TD
  • DELETE does not take a payload
  • Introduce a subscription id resource on which the unsubscribe operation is executed
  • revert span->div
  • normative text needs to be outside of examples

@mlagally
Copy link
Contributor Author

Created issue #329 to address the issue wrt. normative text for the entire spec.

@mlagally
Copy link
Contributor Author

mlagally commented Dec 7, 2022

Arch call on Dec 7th:
Text is an improvement, agree to merge.

@mlagally mlagally merged commit 3e42c5d into main Dec 7, 2022
When a listener receives an event message in a data payload, in many cases it just acknowledges the
successful reception of the event.
Additionally, it may provide a dataResponse payload, which provides a back-channel that can be used
to communicate further details from the consumer to the WebThing.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebThing -> Web Thing

<p>
A Webhook is similar to a callback mechanism in programming languages. Consumers can
subscribe to events they are interested in by registering a listener with the event endpoint.
When the event condition occurs, the WebThing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebThing -> Web Thing

Comment on lines +2871 to +2872
Depending on the deployment scenarios and integration requirements for existing consumers, it may be
required to use specific data payload formats (e.g. Cloud Events).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true for Webhooks in general, but not for profiles which are designed for greenfield deployments.

This specification should define its own payload format, see #258

<p><span class="rfc2119-assertion" id="http-webhook-profile-protocol-binding-events-1">
Depending on the use case, a single listener for multiple things and multiple
event types MAY be used.</span>
</p>
<p><span class="rfc2119-assertion" id="http-webhook-profile-protocol-binding-events-2">
To minimize network traffic, the same listerner MUST NOT perform multiple subscriptions to the same listener.</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listerner -> listener

This assertion doesn't make sense. Why would a listener subscribe to a listener?

@@ -3046,26 +3054,22 @@ <h4><code>subscribeevent</code></h4>
<code>Accept</code> header set to <code>application/json</code>
</li>
<li>
A body with a subscription payload, serialized in JSON.
A body with a subscription request payload, serialized in JSON.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also mention Content-Type header set to application/json, since the request has a JSON payload.

<li>Method set to <code>POST</code></li>
<li>URL set to the URL of the <code>Event</code> resource</li>
<li>Method set to <code>DELETE</code></li>
<li>URL set to the URL of the <code>Event</code> resource followed by the <code>subscriptionId</code> of a valid subscription</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example 44 uses subscriptionID, but this says subscriptionId.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should the response be? I would suggest 204 No Content, as with the cancelaction protocol binding in the HTTP Basic Profile. This would also mean no Accept header is needed on this request.

Comment on lines +3093 to +3094
<code>Accept</code> header set to <code>application/json</code>
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the Accept header is necessary here is it?

</ul>

<pre class="example" title="Unsubscribe using subscriptionId">
DELETE /things/lamp/events/overheated/1234-4544-1211 HTTP/1.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assertion http-webhook-profile-protocol-binding-subscribeevent-4b says that the subscription ID should be sent in the payload, but here it is included in the URL. I agree it should be in the URL.

Comment on lines +3214 to +3215
<code>Accept</code> header set to <code>application/json</code>
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responses don't need Accept headers.

</p>

<pre class="example" title="Unsubscribe all using callbackURI">
DELETE /things/lamp/events HTTP/1.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request URL should include the subscription ID.

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 this pull request may close these issues.

2 participants