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

Notification API: pass event type in HTTP path instead of query #198

Merged
merged 8 commits into from
Jun 7, 2021

Conversation

farshidtz
Copy link
Member

@farshidtz farshidtz commented Jun 7, 2021

Based on refactor-3, from #196.

This is a change to the API, a part of the proposal in #159.

The splitting of the event affordance from one to three (#196) resulted in a strange-looking API specification. In other words, this PR is mostly a change to make the spec look nicer.

Functionality-wise, this change will make it harder / less efficient if a client wants to subscribe to a subset of events (more than one, but not all). HTTP/2 multiplexes such subscriptions but the server and clients still need to allocate additional memory to handle concurrent subscriptions.

Thinking about MQTT as a future extension, this change makes it much easier to have a 1-to-1 mapping. In MQTT, there is no way to subscribe to a subset of events in a single SUB operation. It would be either one (e.g. topic things/create) or all (e.g. topic: things/+). Multiple subscriptions in MQTT share a single connection, similar to SSE over HTTP/2.

In general, I am in favor of this change.


The current API:

/events{?type,diff}

e.g. expansion:

  • /events?type=create&diff=true one event
  • /events?diff=true all events

is being changed to:

/events{/type}{?diff}

e.g. expansion:

  • /events/create?diff=true one event
  • /events?diff=true all events

Preview | Diff

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.

3 participants