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

Introduce a concept of a room version specification #1669

Closed

Conversation

Projects
None yet
4 participants
@turt2live
Copy link
Member

commented Sep 10, 2018

Rendered: see 'docs' status check


The "Room Specification" (or "Room Version Specification") is the specification that defines which room versions do what and are intended to be documents which speak the truth about how rooms operate under the hood.

The approach taken here is a bit different than other specifications. For starters, the specification is versioned in this project instead of relying on the matrix.org repository to track compiled HTML. This is done for a couple reasons, the first being we're still developing the v1 specification while concurrently making a v2 spec and the second being trying to reduce the reliance on matrix.org's repository for specifications.

Because the room spec is built into versions, some changes needed to be made. The targets.yaml now has a special syntax for indicating what version something is at, and the changelog generator can handle rendering different versions of the same changelog (as parsed from the RST). Some additional work has been put in to the changelog parsing to allow us to reference the v1 room spec as "v1" without having to sacrifice clarity in the changelog headings.

Finally, this moves the identifier grammar as it stands into the v1 room spec. This is in anticipation of the v2 room spec having a different set of grammar. Event schemas haven't been migrated here while the MSC to change them is in-flight.

There are a few open questions at this point:

  • Do we move state resolution into here?
  • Do we move the auth rules into here?

Note: this does not introduce the concept of versioned schemas (tabs) that I was previously working with. There's currently no use for them, so they are shelved elsewhere.

Introduce a concept of a room version specification
The "Room Specification" (or "Room Version Specification") is the specification that defines which room versions do what and are intended to be documents which speak the truth about how rooms operate under the hood.

The approach taken here is a bit different than other specifications. For starters, the specification is versioned in this project instead of relying on the matrix.org repository to track compiled HTML. This is done for a couple reasons, the first being we're still developing the v1 specification while concurrently making a v2 spec and the second being trying to reduce the reliance on matrix.org's repository for specifications.

Because the room spec is built into versions, some changes needed to be made. The `targets.yaml` now has a special syntax for indicating what version something is at, and the changelog generator can handle rendering different versions of the same changelog (as parsed from the RST). Some additional work has been put in to the changelog parsing to allow us to reference the v1 room spec as "v1" without having to sacrifice clarity in the changelog headings.

Finally, this moves the identifier grammar as it stands into the v1 room spec. This is in anticipation of the v2 room spec having a different set of grammar. Event schemas haven't been migrated here while the MSC to change them is in-flight.

There are a few open questions at this point:
* Do we move state resolution into here?
* Do we move the auth rules into here?

Note: this does not introduce the concept of versioned schemas (tabs) that I was previously working with. There's currently no use for them, so they are shelved elsewhere.

@turt2live turt2live added this to In review (just the PRs) in August 2018 r0 via automation Sep 10, 2018

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2018

Just for a bit of perspective on the plan here: Assuming this seems sensible, the event schemas are the next thing to get migrated into the room spec. There's a few open issues on matrix-doc about how the schemas are unclear, should be shared, etc and with the potential of those schemas changing it seems important to version them out of the CS/SS/AS APIs.

@turt2live turt2live requested a review from matrix-org/spec-core-team Sep 10, 2018

@ara4n

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

i'm a bit torn. i can see an argument for gathering all the room semantics into a single place, especially everything which might change between given versions of a room - e.g. state res, auth rules. Perhaps one could separate them into "stuff for client implementers" and "stuff for server implementers".

However, i also see an argument for keeping scary state res stuff firmly in the s2s API, where only server implementers will be looking, given client implementers really shouldn't have to care about that and it will only make the spec look even more scary and imposing.

Event schemas haven't been migrated here while the MSC to change them is in-flight.

which one are you thinking about? extensible events? i'd definitely not block refactoring the event schemas into a separate section of the spec on extensible events, as there simply isn't a hard blocker there?

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2018

Event schemas being the introduction of hashes as event IDs - it's an excuse to merge the different folder structures holding events and make it all happy again.

@richvdh

This comment has been minimized.

Copy link
Member

commented Sep 11, 2018

Having spent all of 30 seconds skimming through the generated docs, this looks like a great start. Thanks Travis!

I'm afraid I'm not entirely convinced about moving all the grammars into the room spec, since many of them (eg, user id, server_name) are used in contexts entirely unrelated to rooms. (If we do come to replace the format of the user id, we're going to have to spend a while figuring that out).

I also share the indecision about moving state res, though I think that, given it's about the only thing that we currently know is going to be different between room v1 and room v2, it's sort of the whole point of having a separate room spec. I wonder if we could resolve the problem with a "below this point there be dragons which only server implementors will need to care about" section which is not expanded unless you request it. (Or indeed, multiple such sections.)

@ara4n

This comment has been minimized.

Copy link
Member

commented Sep 11, 2018

Yeah, i think 'here be dragons' could well be okay, as long as the CS-facing stuff is up front.

Shuffle the grammar definitions; Move state res and auth rules
The not-room grammars have been moved back to the appendices.
@turt2live

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

@richvdh @ara4n when you both get a chance, please take another look at this. It should be more room related (with plenty of "here by dragons" warnings) and less ignorant copy/paste.

@uhoreg
Copy link
Member

left a comment

I didn't review the Python bits, and I'm assuming that your copy-and-pastes are accurate. Overall, looks good, with a few comments/changes.

.. sectnum::


Room version grammar

This comment has been minimized.

Copy link
@uhoreg

uhoreg Oct 5, 2018

Member

IMHO, the grammar should go below the "Other room versions" section

This comment has been minimized.

Copy link
@turt2live

turt2live Oct 10, 2018

Author Member

This is done for a couple reasons:

  • It makes tricking the renderer into thinking the first couple lines of a subsequent document are part of the "Other room versions" section easier
  • I somewhat felt that describing what a room version was before listing others ones made sense, given the room version grammar itself is not a function of the room version. I'm not married to this idea though.

I'll look into making the rendering a bit safer though to shuffle the sections. Relying on one document coming after another isn't the best idea in the first place.

upon through new algorithms or rules, "room versions" are employed to
manage a set of expectations for each room.

The format of a room version is described in detail in the `Appendices`_,

This comment has been minimized.

Copy link
@uhoreg

uhoreg Oct 5, 2018

Member

Room versions is no longer described in the appendices. Also bits of this paragraph seem redundant given that it's repeated in the previous section.

Size limits
-----------

The complete event MUST NOT be larger than 65535 bytes, when formatted as a

This comment has been minimized.

Copy link
@uhoreg

uhoreg Oct 5, 2018

Member

It seems to me that the size limit for the entire event should be left here so that it's common for all versions, so that clients and servers can check the size before needing to do any processing.

@uhoreg

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

Also, there are some Docutils System Messages at the bottom of https://2098-24998719-gh.circle-artifacts.com/0/root/project/scripts/gen/rooms/v1.html

@turt2live turt2live self-assigned this Oct 5, 2018

@richvdh richvdh self-requested a review Oct 10, 2018

@richvdh
Copy link
Member

left a comment

Looks good to me, modulo the nit below and the things @uhoreg has already identified.

Is the plan to move all of the m.room.* event definitions in here too?

details contained here, and can safely ignore their presence.


The algorithms defined here should only apply to version 1 rooms. Other algorithms

This comment has been minimized.

Copy link
@richvdh

richvdh Oct 10, 2018

Member

v2

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Oct 10, 2018

Is the plan to move all of the m.room.* event definitions in here too?

@richvdh it could be. This is a very good question for "what do we actually want in a room version spec". Listing the events that people could see makes sense to me, although I do fear that there'd be a lot of duplication between room versions.

@richvdh richvdh removed their assignment Oct 12, 2018

@richvdh richvdh moved this from In Progress: Planned Project Work to Review in Superceded by https://github.com/orgs/matrix-org/projects/8 Nov 1, 2018

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2018

ftr I'm adapting this to be the spec component of #1759. We can come to a conclusion about identifiers, event representation, etc as needed.

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

I'm going to also close this for now pending a new PR to get a clean history.

@turt2live turt2live closed this Jan 4, 2019

August 2018 r0 automation moved this from In review (just the PRs) to Done (this list will be incomplete) Jan 4, 2019

Superceded by https://github.com/orgs/matrix-org/projects/8 automation moved this from Review to Done - Operations Jan 4, 2019

@turt2live turt2live removed this from Done (this list will be incomplete) in August 2018 r0 Jan 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.