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

Update Synapse default room version (6 -> 9) #1327

Merged
merged 4 commits into from
Feb 23, 2022

Conversation

altsalt
Copy link
Contributor

@altsalt altsalt commented Oct 12, 2021

From the Synapse 1.43.0 release highlights:

Asks clients to prefer room version 9 when creating restricted rooms (#10772), via the API defined in MSC3244: room version capabilities.

From the [Synapse 1.43.0 release highlights](https://matrix.org/blog/2021/09/21/synapse-1-43-0-released):
> Asks clients to prefer [room version 9](matrix-org/matrix-spec-proposals#3375) when creating restricted rooms ([#10772](matrix-org/synapse#10772)), via the API defined in [MSC3244: room version capabilities](matrix-org/matrix-spec-proposals#3244).
@aaronraimist
Copy link
Contributor

If you read that sentence again you'll see that they have not changed the default room version. They have only implemented the room version capabilities API to allow clients that wish to specifically use the features provided by v9 to create v9 rooms.

The default room version in the Synapse and the spec remains at 6.

@altsalt
Copy link
Contributor Author

altsalt commented Oct 23, 2021

I see. Oddly, the stable spec only lists through version 6, whereas the unstable has it through version 7, with 6 still being recommended. That said, the space-wide access requires version 9 and upgrading room is not the best experience for room participants, so creating them as version 9 initially makes sense. Still, going with the advised stable room versions makes sense, so until that all changes, should I just close this PR?

@aaronraimist
Copy link
Contributor

aaronraimist commented Oct 23, 2021

If you create a new room in a space with a modern version of Element, it will be version 9 today with no configuration changes. And you are welcome to set matrix_synapse_default_room_version: "9" in your vars.yml on your own server if you want every new room to be v9.

But yes for the moment this should be closed or left open in with the knowledge that it won't be merged for a while. It will probably be ~6 months until version 9 is the default room version.

Version 7 likely become the default in the near future. I am planning on making a proposal to change the default in the spec to version 7 as soon as ruma/ruma#738 is merged.

@altsalt
Copy link
Contributor Author

altsalt commented Oct 24, 2021

Another thought crossed my mind, shouldn't this be set to a default value of blank as in the sample config? A friend pointed out that while there is a hard-coded default there is a check for a user set value before falling back to it.

@altsalt
Copy link
Contributor Author

altsalt commented Oct 24, 2021

Oh, and I meant to mention that I saw the updated list of room versions which included 7 but not 8 or 9 here.

@spantaleev
Copy link
Owner

We set it to a value explicitly instead of setting it to null (which leads to default_room_version: null in homeserver.yaml), because if we set it to null, the following Synapse code here complains:

Error in configuration:
Unknown default_room_version: None, known room versions: ['1', '2', '3', '4', '5', '6', 'org.matrix.msc2176', '7', '8', '9', 'org.matrix.msc2716v3']

When it comes to Synapse, a null value is not the same as a missing value. If the default_room_version setting is undefined in homeserver.yaml, then Synapse would use its default version: https://github.com/matrix-org/synapse/blob/v1.45.1/synapse/config/server.py#L354

A null value leads to the aforementioned error, however.


We can avoid setting default_room_version to null with code like this in homeserver.yaml:

{% if matrix_synapse_default_room_version is not none %}
default_room_version: {{ matrix_synapse_default_room_version|to_json }}
{% endif %}

However, this is somewhat ugly.


Synapse complaining about a null value for default_room_version is something that can be fixed though. We wouldn't need to maintain our matrix_synapse_default_room_version variable in sync with the Synapse default room version anymore, if we patch Synapse like this:

default_room_version = config.get("default_room_version", DEFAULT_ROOM_VERSION)

if default_room_version is None:
    default_room_version = DEFAULT_ROOM_VERSION

... but it until such a thing gets merged and propagates into a few Synapse versions, we need to retain our code of explicitly setting default_room_version to a valid value.

@altsalt
Copy link
Contributor Author

altsalt commented Jan 18, 2022

I haven't forgotten about this and have been keeping it open to see if movement can be made upstream in regards to the latter thought.

That said, there has been more discussion about defaulting to room v9: matrix-org/matrix-spec-proposals#3589

@aaronraimist
Copy link
Contributor

Yeah it should finally happen in the next week or two.

@aaronraimist
Copy link
Contributor

This can be merged now. The spec now says v9 is the default and the next version of Synapse will use v9 as the default.

@spantaleev spantaleev merged commit dae5240 into spantaleev:master Feb 23, 2022
@spantaleev
Copy link
Owner

Thanks for pinging us!

It may have been better to wait an extra week, until Synapse makes the switch, but.. I've merged it now.

I've been using room version 9 as the default for months. I'm sure others have done the same, so it doesn't seem risky switching.

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