Improve documentation around EDUs and PDUs #1463
Conversation
Clarify fields, improve examples, and make the tables in the spec be generated rather than duplicated.
| example: {"key": "value"} | ||
| properties: |
There was a problem hiding this comment.
Note: these are defined because many clients rely on them, and they are relatively generic. They are all optional though.
| os.path.join(matrix_doc_dir, "api/application-service/definitions"): "as", | ||
| os.path.join(matrix_doc_dir, "api/client-server/definitions"): "cs", | ||
| #os.path.join(matrix_doc_dir, "api/identity/definitions"): "is", | ||
| #os.path.join(matrix_doc_dir, "api/push-gateway/definitions"): "push", |
There was a problem hiding this comment.
push and identity don't have definitions, so we shouldn't explode
We fixed the EDU, so we don't need this comment.
This is useful for when we start defining event schemas. This also has a sanity check for ensuring the directory exists, allowing the IS and push API paths to be uncommented.
| age_ts: | ||
| type: integer | ||
| format: int64 | ||
| description: The POSIX timestamp this message was sent at in milliseconds. |
There was a problem hiding this comment.
Its probably a bug if age_ts is coming down the wire, as its an estimate used internally in synapse to track what the age param should be (and isn't the same as origin_server_ts)
There was a problem hiding this comment.
Looking at it again, this seems to not make it down the wire but where I was looking at the time does imply it does. Will remove.
| content: | ||
| type: object | ||
| description: The content of the ephemeral message. | ||
| required: ['edu_type', 'origin', 'destination', 'content'] |
There was a problem hiding this comment.
Empirically we don't send either origin or destination across federation
There was a problem hiding this comment.
All the code seems to point to both being sent, unless something I didn't see somehow drops them? I suppose I could actually test it somehow.
There was a problem hiding this comment.
can confirm: apparently synapse does not send those fields. Will flag as optional.
There was a problem hiding this comment.
Cool. TBH I'd omit them entirely to avoid confusion, since its one less thing that we'd need to check/auth (to avoid the origin in the content being different from the origin in the HTTP request headers, etc.)
|
Is it possible to somehow reuse the event swagger definitions for unsigned PDUs? Maybe by having signed events extend unsigned PDUs? It seems a shame to duplicate the definitions of the fields |
|
Which duplication in particular? It should be possible to have things extend the unsigned PDU, although I don't see where this PR can benefit from doing that. |
|
Have updated this. The python stuff adds a way for definitions (schemas) to appear in the spec without the formal structure of an HTTP API. It does this by adding a new generated section and populating the variable space with the values, leading to definitions being available in the RST. |
I guess I'm assuming that we already have a signed PDU schema somewhere already? |
|
The only signed PDU schema I could find was a table in the RST, which doesn't really help anyone. (also have removed the unused EDU fields) |
|
Ah, pdu.yaml already inherits from unsigned_pdy.yaml |
Signed-off-by: Stuart Mumford <stuart@cadair.com>
Rendered: see 'docs' commit status
Clarify fields, improve examples, and make the tables in the spec be generated rather than duplicated.