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

Generate yaml config for sydent #388

Closed
wants to merge 26 commits into from
Closed

Conversation

Azrenbeth
Copy link
Contributor

@Azrenbeth Azrenbeth commented Sep 9, 2021

Depends on #385

Add a script to generate a YAML configuration file for Sydent.
(NOTE: this is currently not read anywhere, this will be added later)

Advantages of YAML

  • More similar to configuring Synapse
  • Not configparser, which has more confusing structure and a confusing get() method

Why I did it like this:

  • Similar to Synapse
  • Config generation code is closer to where it is parsed, rather than centralised
  • Every option is now documented, including whether or not a value is required for
    Sydent to function.
  • Names of things now closer to what config option does

Things to note

  • The deprecated template options do not have a YAML equivalent. Admins should
    instead create a default brand template folder at the same time as migrating to YAML

Things I was unsure about

  • Should 'general' be split up?
  • Should each section have it's own block of yaml? This would allow easier schema validation.
  • Does anything in Sydent want renaming?
  • What form of string formatting should be used? It currently has some config options
    with {arg} and some with %(arg)s formatting. Should we only use one type or is this OK?

Azrenbeth added 25 commits September 8, 2021 10:52
Configuration handling will be slowly moved over to this SydentConfig
class and away from the rest of the code.
The SydentEd25519 class is no longer needed as all it did was
handle the signingkey config option before (which is now done
in CryptoConfig)

In the current setup, if no key is present a new one is generated
and saved back to file. This is a special case for the crypto config
and so the line `return self.crypto.save_key` isn't more general.

In the future this might not be needed (e.g. by having sydent only
do key generation when it first creates a config file for you, rather
than everytime one is not found) and so this special casing can be
removed.
test_jinja_templates used `("email", "email.verification_template")`
for this argument, however this config option isn't mentioned
anywhere else in the code (or even in the test's config) so it's ok
to remove it
- Move config file and dict handling over to SydentConfig
- Alter all places where Sydent object is constructed
- Remove parse_cfg_bool and set_from_comma_set_string from sydent.py
  as these functions were only used for config parsing
- Remove save_config from sydent.py as this can now be done from
  SydentConfig
migrating_config.yaml has each YAML option set to the old-style
config option that it replaces.
@Azrenbeth
Copy link
Contributor Author

Gone down another route

@Azrenbeth Azrenbeth closed this Sep 20, 2021
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.

None yet

1 participant