MSC1753: client-server capabilities API #1753
Conversation
|
Simple and straightforward - lgtm! |
proposals/1753-capabilities.md
Outdated
| query, where a server would return all possible supported operations. The | ||
| problem with this is that it may add load to the server, and increase network | ||
| traffic, by returning a large number of features which the client may have no | ||
| interest in. |
There was a problem hiding this comment.
This sounds slightly spurious to me, tbh. You'd need quite a large set of capabilities before it'd be noticeable I'd have thought (especially when using compression). I also wouldn't expect it be queried very often by clients, so compared to /sync and co I think the traffic/load would be negligible?
OTOH, I'm not against using a POST, though I kinda expect a lot of folks to ask why they can't simply do a GET (I could certainly see myself wanting to query everything to debug what's going on)
There was a problem hiding this comment.
My concern is less about network traffic and more about a world where a server is divided up into microservices so has to go and ask lots of other bits of the system for answers on the capabilities - or even just do database queries or whatever.
|
Broadly LGTM, other than I'm wondering slightly about having |
|
it generally lgtm. i think it'd also be nice to support GET to discover what caps are around as an fyi (even if we don't know what to do with them), even though clients would in practice typically POST to do discover specific caps. A nagging part of my brain is wondering whether should put these as state in a server room (same as the server msgs room?) to avoid creating yet another custom API and to get realtime sync for free, etc. |
Surely clients are going to use the GET if it's available, thus defeating the point of the POST. I don't feel that strongly about whether it should be GET or POST, but having both seems to be the worst of all worlds. |
that sounds much more complicated to implement. Why is a custom API so much worse than custom event types? |
because it’s another set of endpoints to be specced, impl’d and maintained - and a new section to /sync if we want it pushed to the client in realtime. i’m surprised it seems more complicated, but i may be missing something? To be clear: i am just trying to do the thought experiment here to ensure we evaluate both options sufficiently. |
|
All I want for Christmas is threading.
one endpoint, which seems to compare very favourably to one custom event type which also has to be specced, impl’d and maintained.
Let's suppose we change the configuration on the server so that users can no longer change their passwords. That means that the server needs to (a) spot that config change has been made, which may be non-trivial, and then (b) create a state event for every single user who has ever used the server so that it's ready for the client next time it syncs. That process may take a long time, so we need to be able to resume it halfway through. As an alternative to (b), maybe we wait until the client syncs before creating a new event, but then that feels very similar to adding a new section to /sync. Compare that with a simple endpoint that the client can go and check when it cares about something? |
|
Based on the feedback here, I'm going to update the proposal to suggest a simple |
|
It seems a bit unfair to call a FCP on this over Christmas, but if folks could let me know if they have anything else to add that would be helpful, so we can FCP it in January. |
|
ok folks I'd like to propose an FCP for this. |
|
@mscbot fcp merge |
|
Team member @richvdh has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
mscbot
added
proposed-final-comment-period
r0/versions isn't a thing Co-Authored-By: richvdh <1389908+richvdh@users.noreply.github.com>
r0/versions isn't a thing Co-Authored-By: richvdh <1389908+richvdh@users.noreply.github.com>
r0/versions isn't a thing Co-Authored-By: richvdh <1389908+richvdh@users.noreply.github.com>
|
lgtm. @mscbot reviewed |
mscbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
mscbot
added
finished-final-comment-period
and removed
final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. |
Original proposals: * #1753 * #1804 Implementation proof: * matrix-org/synapse#4472 * matrix-org/matrix-js-sdk#830 There is one change to MSC1753 which is included in this commit. MSC1804 remains unchanged. In the original proposal, the change password capability being present was an indication that password changes were possible. It was found that this doesn't really communicate the state very well to clients in that lack of a capability (or a 404, etc) would mean that users would erroneously not be able to change their passwords. A simple boolean flag was added to assist clients in detecting this capability.
turt2live
added
spec-pr-in-review
|
Merged via #1829 🎉 |
Rendered