Skip to content

Conversation

V02460
Copy link
Contributor

@V02460 V02460 commented Oct 31, 2024

Adds a JSON Schema description of the Synapse configuration to the repo, together with a script generating docs/usage/config_documentation.md from it.

This has many advantages, one can:

  • Validate Synapse configuration files against the schema.
  • Keep configuration and its documentation closer together, avoiding de-synchronization.
  • Keep a uniform documentation style.
  • Represent enforceable constraints between config values.
  • Choose to reduce the amount of hand-rolled config validation code.
  • Have IDE support like keyword suggestion and linting when editing a Synapse configuration.

I was surprised how aligned the expressiveness of the schema file is with the current infos from the Synapse documentation. Besides added types and defaults, some whitespace differences, and minor changes to wording and ordering, the config documentation remains mostly unchanged. But still, it is no perfect match, with the most important restrictions being:

  1. No native sections for config options (the script patches those in).
  2. No comments in examples. Might be handled as part of the description.
  3. No native JSON Schema keyword representing the “Added in Synapse <version>” annotations (added to the plain-text description).

I expect there will be some feedback to work into this MR before it can get merged. I’m especially thinking about CI integration, JSON Schema IDs, types and defaults, and discussions about workflow – as well as a good amount of fixes due to the size of the change. Looking forward to your comments :)

Example

Running the script

scripts-dev/gen_config_documentation.py schema/synapse-config.schema.yaml > docs/usage/configuration/config_documentation.md

on a schema containing the following fragment

"soft_file_limit": {
  "type": "integer",
  "description": "Set the soft limit on the number of file descriptors synapse can use. Zero is used to indicate synapse should set the soft limit to the hard limit.",
  "default": 0,
  "examples": [3]
}

yields the corresponding Synapse documentation as Markdown:

### `soft_file_limit`

*(integer)* Set the soft limit on the number of file descriptors synapse can
use. Zero is used to indicate synapse should set the soft limit to the hard
limit. Defaults to `0`.

Example configuration:
```yaml
soft_file_limit: 3
```

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.
  • Code style is correct
    (run the linters)

@V02460 V02460 requested a review from a team as a code owner October 31, 2024 17:26
@CLAassistant
Copy link

CLAassistant commented Oct 31, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot deployed to PR Documentation Preview November 5, 2024 14:09 Active
@reivilibre reivilibre requested a review from a team November 11, 2024 14:21
@MadLittleMods MadLittleMods requested a review from a team November 11, 2024 16:06
@reivilibre reivilibre requested review from reivilibre and removed request for a team November 13, 2024 10:13
@github-actions github-actions bot deployed to PR Documentation Preview November 13, 2024 22:29 Active
@reivilibre
Copy link
Contributor

reivilibre commented Nov 13, 2024

I reviewed the diff of the config documentation and fixed up some accidental differences that I saw, as well as some other minor improvements (IMO).

Due to time constraints I have yet to deal with these issues:

  • suspicious defaults
    • public_baseurl
    • databases Defaults to {}.
    • session_lifetime
    • refresh_token_lifetime
    • nonrefreshable_access_token_lifetime
    • room_list_publication_rules (should be "*"?)
    • alias_creation_rules (should be "*"?)

Due to a problem with my diff viewer and long lines (???), some descriptions were cut off whilst I was reading them and I have not yet re-reviewed those fragments:

  • remote_media_download_burst_count
  • remote_media_download_per_second
  • media_retention
  • For modern Android devices the notification con...
  • Ensure the main process and all pusher workers are r...
  • and it should be listed...
  • This option defaults to off, enable it by provid...

I thought the section on 'resources' in the listeners section looked suspicious but did not thoroughly re-review yet.


Questions (aimed at the team):

  • should we convert the JSONSchema document itself to YAML?
    That would allow writing comments as well as multiline strings.
    Currently the markdown strings inside the JSON document are a little bit awkward to edit, since they have to be single-line.

Thoughts:

  • string|null might look nicer as string or null.
  • I'd like to try avoiding throwing too much JSON in the reader's face:
    • 'Defaults to []' might look nicer as 'Defaults to an empty list'
    • 'Defaults to {}' might have a nicer alternative?
    • Maybe we should follow what we used to do and use the word 'none' instead of 'null'...?
    • 'list' instead of 'array'?
    • maybe there's a word for 'object' that would suit better
  • In url_preview_url_blacklist and some others, it would probably be nice to preserve the comments in the example. We can probably add some io.element.yaml_example field to the schema with verbatim YAML for such cases, which should be fairly rare.

Things missing from this PR that I think are needed:

  • some options are missing descriptions and this shows up as MISSING DESCRIPTION. We should add descriptions for those.
  • The config documentation generation should be checked in CI — to make sure the schema is valid and that the doc doesn't fall out of sync by accidental manual edits
  • missing extra docs on enable_authenticated_media (due to a concurrent PR)

Things missing from this PR that can probably wait:

  • Synapse should enforce the JSONSchema validations on startup (both because this would be useful and because this will help ensure they don't fall out of sync)

Risks:

  • if the schema is wrong and we start validating it in Synapse, we might break some servers' ability to start up

If we accepted this PR now but for some reason didn't like it later:

  • We could regenerate the config documentation, then remove the schema and generator. We'd be back in the same place as now, before accepting the PR.

@github-actions github-actions bot deployed to PR Documentation Preview May 27, 2025 12:00 Active
@github-actions github-actions bot deployed to PR Documentation Preview May 27, 2025 12:05 Active
@V02460 V02460 requested a review from anoadragon453 May 27, 2025 12:54
@github-actions github-actions bot deployed to PR Documentation Preview May 27, 2025 13:08 Active
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

A few minor things, but we're in the home stretch now!

Co-authored-by: Andrew Morgan <[email protected]>
@github-actions github-actions bot deployed to PR Documentation Preview May 30, 2025 09:13 Active
@github-actions github-actions bot deployed to PR Documentation Preview May 30, 2025 09:18 Active
@github-actions github-actions bot deployed to PR Documentation Preview May 30, 2025 10:00 Active
@V02460 V02460 requested a review from anoadragon453 May 30, 2025 10:14
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Happy to say this now LGTM! I double-checked the generated config documentation vs. the hand-written one and all options seem to be present.

Thanks again for your hard work on putting this together, and keeping it updated even through the long review cycles ❤️

@anoadragon453
Copy link
Member

Note that merging this PR will require updates for any other currently-open PR that modifies the handcrafted configuration documentation. I'll do my best to spread the word around so contributors are aware.

Before merging, I'll add one more commit to this PR that adds a section to the documentation README explaining how the Configuration Manual is now built.

@github-actions github-actions bot deployed to PR Documentation Preview June 3, 2025 09:29 Active
@anoadragon453 anoadragon453 merged commit fae72f1 into element-hq:develop Jun 3, 2025
22 checks passed
erikjohnston pushed a commit that referenced this pull request Jun 10, 2025
…chema. (#18522)

Follows: #17892, #18456

<ol>
<li>

Add config doc generation command to lint.sh 

</li>
<li>

Add missing `user_types` config schema 

</li>
</ol>

---------

Signed-off-by: Olivier 'reivilibre <[email protected]>
erikjohnston pushed a commit that referenced this pull request Jun 10, 2025
Follows: #17892 <!-- -->

<ol>
<li>

Config documentation CI: fix not failing if changes are outstanding 

</li>
</ol>


Shown to work at :
https://github.com/element-hq/synapse/actions/runs/15532406886/job/43724019104?pr=18528

---------

Signed-off-by: Olivier 'reivilibre <[email protected]>
erikjohnston pushed a commit that referenced this pull request Jun 10, 2025
…chema. (#18522)

Follows: #17892, #18456

<ol>
<li>

Add config doc generation command to lint.sh 

</li>
<li>

Add missing `user_types` config schema 

</li>
</ol>

---------

Signed-off-by: Olivier 'reivilibre <[email protected]>
Michael-Ixo pushed a commit to ixoworld/synapse that referenced this pull request Jun 25, 2025
- Improvements to generate config documentation from JSON Schema file. ([\element-hq#18522](element-hq#18522))

- Add support for [MSC4155](matrix-org/matrix-spec-proposals#4155) Invite Filtering. ([\element-hq#18288](element-hq#18288))
- Add experimental `user_may_send_state_event` module API callback. ([\element-hq#18455](element-hq#18455))
- Add experimental `get_media_config_for_user` and `is_user_allowed_to_upload_media_of_size` module API callbacks that allow overriding of media repository maximum upload size. ([\element-hq#18457](element-hq#18457))
- Add experimental `get_ratelimit_override_for_user` module API callback that allows overriding of per-user ratelimits. ([\element-hq#18458](element-hq#18458))
- Pass `room_config` argument to `user_may_create_room` spam checker module callback. ([\element-hq#18486](element-hq#18486))
- Support configuration of default and extra user types. ([\element-hq#18456](element-hq#18456))
- Successful requests to `/_matrix/app/v1/ping` will now force Synapse to reattempt delivering transactions to appservices. ([\element-hq#18521](element-hq#18521))
- Support the import of the `RatelimitOverride` type from `synapse.module_api` in modules and rename `messages_per_second` to `per_second`. ([\element-hq#18513](element-hq#18513))

- Remove destinations from sending if not whitelisted. ([\element-hq#18484](element-hq#18484))
- Fixed room summary API incorrectly returning that a room is private in the room summary response when the join rule is omitted by the remote server. Contributed by @nexy7574. ([\element-hq#18493](element-hq#18493))
- Prevent users from adding themselves to their own user ignore list. ([\element-hq#18508](element-hq#18508))

- Generate config documentation from JSON Schema file. ([\element-hq#17892](element-hq#17892))
- Mention `CAP_NET_BIND_SERVICE` as an alternative to running Synapse as root in order to bind to a privileged port. ([\element-hq#18408](element-hq#18408))
- Surface hidden Admin API documentation regarding fetching of scheduled tasks. ([\element-hq#18516](element-hq#18516))
- Mark the new module APIs in this release as experimental. ([\element-hq#18536](element-hq#18536))

- Mark dehydrated devices in the [List All User Devices Admin API](https://element-hq.github.io/synapse/latest/admin_api/user_admin_api.html#list-all-devices). ([\element-hq#18252](element-hq#18252))
- Reduce disk wastage by cleaning up `received_transactions` older than 1 day, rather than 30 days. ([\element-hq#18310](element-hq#18310))
- Distinguish all vs local events being persisted in the "Event Send Time Quantiles" graph (Grafana). ([\element-hq#18510](element-hq#18510))
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.

5 participants