-
Notifications
You must be signed in to change notification settings - Fork 385
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
Room Versioning #1425
Comments
Last call for comments on this before we go ahead and start implementation next week. |
Are we using integers or strings for room versions? Benefits of strings is that it would make it a) clearer that you can't use ranges, and b) makes it easier to have versions like OTOH, I'm not sure we want to be encouraging weird room versions in the wild |
The error schema implies integers, which I think is a good thing. It's much easier to determine if a room is outdated when it's an integer. |
The wording in this document makes it sound like the actual spec PR may have different "mechanics" than outlined in this document. Specifically, the phrase, "might look like". I just want to be clear that if the actual way that room versions work ends up being different than outlined in the mechanics section of this document, a new spec proposal will be made before a spec PR is merged, right? Other than that, here's some points I'd like clarification on:
For the record, I really like the general approach of this proposal. Worrying about room versioning before worrying about upgrade path seems like the right way to go about things. |
There's not a word about exposing the room version to clients; exposing the join error is only mentioned. Given that the room version may (potentially greatly, in case of event id formats etc.) affect the clients, should this proposal mention where to find the room version in CS API? Or are we deliberately keeping it a server-server thing for now, given that there no actually immediate changes that would need the clients to know about the room version? |
@erikjohnston / @turt2live : let's define them as integers. |
Sorry, that phrasing was really just an artefact of the text being copied from an earlier proposal where things were less certain. Now updated.
It's likely that, as I implement this, I'll discover things that don't work out as planned. If that happens, I'll update the doc and highlight the changes here.
Correct.
Yes, it's likely that some changes that might be supported by a room version bump will require client-side changes to support (an example would be: changing the way bans work so that bans are stored in a separate m.room.ban event rather than an m.room.member event. In order to figure out who has been banned from a room, a client would have to look at a different sort of event).
These are interesting questions which I suspect we haven't thought about hard enough. I think that, for testing future room versions if nothing else, the ability to override the default version is going to be important. I'm hesitant to have an "I only support v1" flag on
An extreme solution to the latter would be to extend the format of Right now, I'm minded to hold fire on this, and make it more of a case-by-case judgement based on exactly what changes are happening: some changes will require significant changes to the C-S API, and others will be invisible to clients, and we should pick a solution accordingly. I'm certainly open to discussion here, though.
Absolutely. I think @turt2live is working on some refactoring of the spec to make this a more tractable thing to do.
In theory, yes. In practice: it's just a particular case of the more general questions about client interaction above. |
@KitsuneRal: clients can observe the room version by looking at the m.room.create event, should they so wish. |
All: thanks for feedback so far. Having it in here where there is a record of it is useful, but I've also created a matrix room (#msc-1425:sw1v.org) if anyone has any /quick/ questions. |
Will clients have to explicitly request |
I'm not sure what the requirements are for this yet, although it is on the list (under r0 and above making it pretty). |
In discussion in #msc-1425:sw1v.org, I became convinced of the merits of making room_version a string. I've updated the doc accordingly, and added a section discussing the allowable grammar. Feedback welcome. [I've also been thinking a bit more about client-side support. I'll update the doc accordingly in due course]. |
I've now added some thoughts on client interaction at https://docs.google.com/document/d/1urKgReoHqxX8R_XtySB17dPi-DZcKhqTEL2_s895Wz0#heading=h.b398exru3iu5. It needs more discussion; feedback once again welcome. |
I don't see much harm in trying to join an incompatible room - after all, I might still be able to read it on another client. On the other hand, filtering the response from |
Basic support for this has now landed in synapse, in matrix-org/synapse#3654. I think it was largely as originally specced, except for some minor tweaks:
The proposal doc has been updated accordingly. |
/me still hopes for the room version in |
To get the version number of the room as per matrix-org/matrix-spec-proposals#1425
This is the spec PR for matrix-org#1425 Room version upgrades are not part of MSC1425. Documented aspects: * room_version on the create event * creating a room with a specific version (useful for testing) * make_join behaviour * error code documentation * grammar of room versions Based upon https://docs.google.com/document/d/1urKgReoHqxX8R_XtySB17dPi-DZcKhqTEL2_s895Wz0/edit
@KitsuneRal: why don't you just get the create event, if you want to know the room version? (I certainly don't think the summary is the right place) |
Well, basically it's yet another call for me, for EACH room in the roomlist - at best, I have to do something like that:
That looks a bit too involved for the initial sync when I really want to show things to the user as soon as possible. And there's never been a case so far when I needed to issue a network request right from processing the response from another network request. Not that libQMatrixClient couldn't do that, but I don't want to mess with special cases due to race conditions between the sync in step 1 and the sync in step 3 (what if the user left some room right before me sending a request for |
I don't quite understand why you wouldn't get the m.room.create in the first sync? |
Because it would be beyond the latest N events I'm getting in the first sync? |
you always receive all state events in the first sync (ignoring lazy-loading members). the limit is only applied to the events in the timeline. |
I suspected that I didn't notice an elephant in the room :) thanks, and sorry for the time taken. |
Merged 🎉 |
the spec pr was #1516 |
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
Documentation: https://docs.google.com/document/d/1urKgReoHqxX8R_XtySB17dPi-DZcKhqTEL2_s895Wz0
PRs: #1516
supercedes #1196.
The text was updated successfully, but these errors were encountered: