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

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Mar 17, 2023

Add stable unstable feature (org.matrix.msc3030.stable) for jump to date before v1.6 is fully supported on a homeserver. Discussion for whether we are okay with moving forward with this -> #15283 (comment)

Related to #15089

Fix element-hq/element-web#24362

Element Web PR to honor org.matrix.msc3030.stable: matrix-org/matrix-react-sdk#10398

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

…ported on Synapse

Related to #15089

Element Web PR to honor `org.matrix.msc3030.stable`: matrix-org/matrix-react-sdk#10398
@MadLittleMods MadLittleMods added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Mar 17, 2023
# 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

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

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

@MadLittleMods MadLittleMods changed the title Add stable unstable version for jump to date before v1.6 is fully supported on Synapse Add stable unstable feature for jump to date before v1.6 is fully supported on Synapse Mar 17, 2023
@MadLittleMods MadLittleMods changed the title Add stable unstable feature for jump to date before v1.6 is fully supported on Synapse Add stable unstable feature for jump to date before v1.6 is fully supported on Synapse Mar 17, 2023
@clokep
Copy link
Member

clokep commented May 12, 2023

@MadLittleMods I think we can close this now that #15089 has landed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jump to date feature cannot be enabled
3 participants