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

Consider a property implementation using SSE as notification mechanism #2

Open
kaifox opened this issue May 8, 2019 · 7 comments
Open

Comments

@kaifox
Copy link
Member

kaifox commented May 8, 2019

Using SSE as notification would bring:

  • Having more similar mechanisms for get/set/subscribe
  • Easy debugging in the browser
  • No deed for additional 'ws://' url

Still we need 2 endpoints (one for get/set, one for the SSEs).
A proposal for endpoint naming:

"/properties/tune" // for get and set
"/properties/tunes" // for subscription (SSE)
@andreacalia
Copy link
Member

Fully agree :)

just a comment about naming, maybe expressing the subscription using url is a bit more generic. In the sense that singular+plural may not be possible to be used in all cases and the property developer must specify then 2 names (not feasible to guess). If we agree on something like /properties/tune/stream this could be generated automatically :)

Just a question out of curiosity, what prevents us to use just 1 endpoint with a different content type like application/stream+json. Is it easier/needed to have 2 endpoints with Spring?

@kaifox
Copy link
Member Author

kaifox commented May 9, 2019

Concerning the different content types on the same endpoint: I do not know. Would have to be checked... I do not know how exactly the final content type is negotiated between server and client. One advantage of haveing two different endpoints is that the server deifnes the content type and when pointing a browser to it, it delivers the right thing.

If we go to the path like structure, indeed then one could have something like you propose:

// for get/set:
"/properties/tune/value"

// for subscription (SSE):
"/properties/tune/stream"

I assume this is the most webish way....

However, thinking a bit further on different properties we might have:

  • Getable
  • Setable
  • Observable
    ... and very often all of them....

taking this into account, one could make it a bit more explicit which operations the property is capable of ...

"/properties/tune/get"
"/properties/tune/set" (or "post")
"/properties/tune/subscribe"

The capability would be defined by the fact that the corresponding enpoint is available...

@michi42
Copy link
Member

michi42 commented May 9, 2019

I think we basically have two options

  1. use SSE + GET/POST on different endpoints
  2. continue using websockets, but also handle get/set through it (so we only have 1 endpoint left)
    I think both have their pros and cons, and I haven't got a strong opinion for either one.

For (1), in principle one could possibly join SSE and GET/POST on one endpoint by means of content negotiation (Accept-Header). However, This is rather brittle and intermediate Proxies might break it, so I'd rather not go for that.

In this case I would rather use 2 endpoints ("value" and "stream" looks OK to me). GET/SET would be distinguished by the allowed HTTP methods on value. You can define an endpoint which only allows e.g. POST to value but not GET, which would be a write-only property. On the other hand, you could only allow GET for a read-only property. Using another method should (by protocol) trigger a "405 Method Not Allowed" type error.

@andreacalia
Copy link
Member

Probably for this use-case, SSE are slightly more suited then websocket I'm starting to think..
Regarding the URL, I think we could try to express them like entities instead of operations, those are carried out by HTTP verbs..

/properties/tune/value GET & POST
/properties/tune/stream SSE

this would allow in the future to add some more endpoints. You could even do a GET /properties/tune or GET /properties/tune/info to get information about the property itself, like read-only, etc..

What do you think?
(I have misfeelings about the /stream but I would not have a better idea hahaha)

@kaifox
Copy link
Member Author

kaifox commented May 10, 2019

Ok ...

Andrea, what misfeelings do you have about the stream?

Could e.g.

/properties/tune/updates 

or

/properties/tune/changes

(as we said we might update only on change)

.. or even

/properties/tune/values

be an option? (I think here the plural would be ok, as it always works)

or

/properties/tune/valueChanges

(I do not like the camelCase too much ....)

just ideas...

@kaifox
Copy link
Member Author

kaifox commented May 10, 2019

... and indeed, I like the extensibility

@andreacalia
Copy link
Member

Sorry, I thought I answered this.. /properties/tune/values looks nice :) it expresses what it does nicely. do you agree? @michi42 @kaifox

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