Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add stable unstable feature for jump to date before v1.6 is fully supported on Synapse #15283

Closed
wants to merge 2 commits into from
Closed

Add stable unstable feature for jump to date before v1.6 is fully supported on Synapse #15283

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea8c61c
Add stable unstable version for jump to date before v1.6 is fully sup…
MadLittleMods Mar 17, 2023
2ef63be
Add changelog
MadLittleMods Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions synapse/rest/client/versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
"fi.mau.msc2815": self.config.experimental.msc2815_enabled,
# Adds a ping endpoint for appservices to check HS->AS connection
"fi.mau.msc2659": self.config.experimental.msc2659_enabled,
# Adds support for jump to date endpoints (/timestamp_to_event) as per MSC3030.
# TODO: Remove when Synapse advertises support for `v1.6` which includes MSC3030
# (https://github.com/matrix-org/synapse/issues/15089)
"org.matrix.msc3030.stable": True,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add stable unstable version (org.matrix.msc3030.stable) for jump to date before v1.6 is fully supported on a homeserver.

org.matrix.msc3030.stable wasn't included in MSC3030 but it seems like pattern that should have been included like MSC2285 as an example.

Are we okay with moving forward with this? Since this is in unstable_features, it seems like we can make any decision although I wish we included it in the MSC for completeness sake.

There is another open thread on the Element Web PR to honor org.matrix.msc3030.stable but it doesn't have any further points.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see us support v1.6 (#15089). I don't think the work is that extensive, although I empathize with it not being directly related to this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I believe the stable flags are also considered poor form and we try not to use them? I forget where I got that from though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I believe the stable flags are also considered poor form and we try not to use them? I forget where I got that from though.)

Seems like a great pattern to me. I wish I had included it on the MSC and seems like it should be followed in other MSC's. Curious of the discussion if you find it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to find the original conversation, but it was something along the lines of:

  • It adds 3 phases to to an MSC implementation (unstable, stable-but-unversioned, versioned); which creates:
    • More implementation work (and more potential for bugs).
    • Harder to deprecate -- you need to separately deprecate the unstable version; then the stable-but-unversioned branches.
  • This shouldn't really be necessary as spec releases are done fairly often now.

Regardless, this isn't in the MSC and this is now in the spec, so I think the way forward is to implement support for the specced version? Otherwise both Synapse and any clients using this will be out of spec.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, the unstable prefix section of an MSC is considered non-normative, which means it can change if needed to support the goals of implementation.

I also can't find the relevant discussion on this, but @clokep's summary is correct. I would also suggest just jumping to stable: the remaining effort of Synapse's converstion to v1.6 appears to be relatively trivial and would be faster to do than managing an unstable-but-stable flag.

Copy link
Contributor Author

@MadLittleMods MadLittleMods Mar 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a related discussion at matrix-org/matrix-js-sdk#2915 (comment) (maybe even a mention of the fabled internal conversation that you're thinking of which you may be able to find given the dates)

For a timeline:

The Gitter stuff got in the way of moving jump to date forward quicker back when that other discussion happened. And now it appears from your inklings that we're probably closer to just having v1.6 in Synapse which works ⏩

But if I was moving faster there, we should have a standard suggested path for these situations. A suggestion of waiting for complete stable isn't satisfactory.

We could have kept the unstable endpoint and feature around longer but there is complexity to support both stable and unstable and we don't have to let breaking unstable and labs features slow us down (more thoughts).

Copy link
Contributor Author

@MadLittleMods MadLittleMods May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1.6 support merged into Synapse via #15089 on 2023-05-12 (t+6M)

I think this illustrates why we can't just wait for things to be stable when you're trying to actively work on something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related thread from @clokep in the #matrix-spec room on how we can standardize these type of things

# Adds support for login token requests as per MSC3882
"org.matrix.msc3882": self.config.experimental.msc3882_enabled,
# Adds support for remotely enabling/disabling pushers, as per MSC3881
Expand Down