Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Create a ListenerConfig object #7681

Merged
merged 3 commits into from
Jun 16, 2020
Merged

Create a ListenerConfig object #7681

merged 3 commits into from
Jun 16, 2020

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Jun 11, 2020

This ended up being a bit more invasive than I'd hoped for (not helped by
generic_worker duplicating some of the code from homeserver), but hopefully
it's an improvement.

The idea is that, rather than storing unstructured dicts in the config for
the listener configurations, we instead parse it into a structured
ListenerConfig object.

This ended up being a bit more invasive than I'd hoped for (not helped by
generic_worker duplicating some of the code from homeserver), but hopefully
it's an improvement.

The idea is that, rather than storing unstructured `dict`s in the config for
the listener configurations, we instead parse it into a structured
`ListenerConfig` object.
@richvdh richvdh requested a review from a team June 11, 2020 22:44
@richvdh
Copy link
Member Author

richvdh commented Jun 12, 2020

this is going to need matrix-org/sytest#888 before it will work correctly: 0 is not, and never has been, a valid setting for tls and compress. I can't see that we need to support it.

@clokep
Copy link
Member

clokep commented Jun 12, 2020

I kicked off the tests again, looks like they're passing now! 👍

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks pretty reasonable overall!

synapse/config/server.py Outdated Show resolved Hide resolved
synapse/app/generic_worker.py Outdated Show resolved Hide resolved
synapse/app/homeserver.py Show resolved Hide resolved
bind_addresses = listener_config["bind_addresses"]
tls = listener_config.get("tls", False)
site_tag = listener_config.get("tag", port)
def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConfig):
Copy link
Member

Choose a reason for hiding this comment

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

I think if any of the comments for the worker version end up in changes we'll need to make them here too.

@richvdh
Copy link
Member Author

richvdh commented Jun 16, 2020

thanks for the review @clokep! ptal?

@richvdh richvdh requested a review from clokep June 16, 2020 10:56
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good (assuming CI passes)!

@richvdh richvdh merged commit 0361932 into develop Jun 16, 2020
@richvdh richvdh deleted the rav/listener_config_obj branch June 16, 2020 11:44
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '03619324f':
  Create a ListenerConfig object (#7681)
  Fix changelog wording
  1.15.1
  Wrap register_device coroutine in an ensureDeferred (#7684)
  Ensure the body is a string before comparing push rules. (#7701)
  Ensure etag is a string for GET room_keys/version response (#7691)
  Update m.id.phone to use 'phone' instead of 'number' (#7687)
  Fix "There was no active span when trying to log." error (#7698)
  Enable 3PID add/bind/unbind endpoints on r0 routes
  Discard RDATA from already seen positions. (#7648)
  Replace iteritems/itervalues/iterkeys with native versions. (#7692)
  Fix warnings about losing log context during UI auth. (#7688)
  Fix a typo when comparing the URI & method during UI Auth. (#7689)
  Remove "user_id" from GET /presence. (#7606)
  Increase the default SAML session expirary time to 15 minutes. (#7664)
  fix typo in sample_config.yaml (#7652)
  Take out a lock before modifying _CACHES (#7663)
  Add option to enable encryption by default for new rooms (#7639)
  Clean-up the fallback login code. (#7657)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants