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

Fix typechecking errors introduced in #14128 #14455

Merged
merged 3 commits into from
Nov 15, 2022
Merged

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Nov 15, 2022

I haven't dug into why we missed this. Best guess is

  • CI green on the PR
  • we updated a dependency on develop to include new stubs/annotations (Twisted?)
  • CI wasn't rerun against that develop again

@DMRobertson DMRobertson marked this pull request as ready for review November 15, 2022 14:02
@DMRobertson DMRobertson requested a review from a team as a code owner November 15, 2022 14:02
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.

Seems reasonable if CI is happy. 👍

@DMRobertson DMRobertson enabled auto-merge (squash) November 15, 2022 14:04
@DMRobertson
Copy link
Contributor Author

Complement is unhappy.

@tuojamie
Copy link
Contributor

You probably can't do the assert self.tls_server_context_factory is not None here because it may be None if certificates have not been configured.

@DMRobertson
Copy link
Contributor Author

tls_server_context_factory is not even None in that situation: it's not set as an attribute. (The existing annotations are unhelpful here.) Point being: it should still fail (with an AttributeError) without the assertion.

so that context_factory works if you don't use TLS
@@ -258,6 +256,9 @@ def __init__(
self._module_web_resources: Dict[str, Resource] = {}
self._module_web_resources_consumed = False

# This attribute is set by the free function `refresh_certificate`.
self.tls_server_context_factory: Optional[IOpenSSLContextFactory] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this might be redundant; I think mypy infers the type of instance attributes if they're set in the constructor like this? 🤷

Comment on lines -224 to -225
tls_server_context_factory: Optional[IOpenSSLContextFactory]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with ABCMeta but I don't think this should have magically an instance attribute?

(What I'm driving at is: why didn't complement fall over on #14128 but did on an earlier version of this PR?)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with ABCMeta but I don't think this should have magically an instance attribute?

Regardless of ABCMeta I would think this would end up as an expected property? But yeah there's no guarantee it actually gets added unless we do it in __init__.

(Using a free function to set it is kind of meh...)

@DMRobertson DMRobertson enabled auto-merge (squash) November 15, 2022 16:07
@DMRobertson
Copy link
Contributor Author

Complement and sytest failures look like a mixture of flakes and needing #14442 to land (...whose automerge is blocked on the typing problems this PR seeks to fix).

@DMRobertson DMRobertson merged commit 258b528 into develop Nov 15, 2022
@DMRobertson DMRobertson deleted the dmr/fix-mypy-again branch November 15, 2022 16:36
bradtgmurray added a commit to beeper/synapse-legacy-fork that referenced this pull request Nov 22, 2022
Synapse 1.72.0 (2022-11-22)
===========================

Please note that Synapse now only supports PostgreSQL 11+, because PostgreSQL 10 has reached end-of-life, c.f. our [Deprecation Policy](https://github.com/matrix-org/synapse/blob/develop/docs/deprecation_policy.md).

Bugfixes
--------

- Update forgotten references to legacy metrics in the included Grafana dashboard. ([\matrix-org#14477](matrix-org#14477))

Synapse 1.72.0rc1 (2022-11-16)
==============================

Features
--------

- Add experimental support for [MSC3912](matrix-org/matrix-spec-proposals#3912): Relation-based redactions. ([\matrix-org#14260](matrix-org#14260))
- Build Debian packages for Ubuntu 22.10 (Kinetic Kudu). ([\matrix-org#14396](matrix-org#14396))
- Add an [Admin API](https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html) endpoint for user lookup based on third-party ID (3PID). Contributed by @ashfame. ([\matrix-org#14405](matrix-org#14405))
- Faster joins: include heroes' membership events in the partial join response, for rooms without a name or canonical alias. ([\matrix-org#14442](matrix-org#14442))

Bugfixes
--------

- Faster joins: do not block creation of or queries for room aliases during the resync. ([\matrix-org#14292](matrix-org#14292))
- Fix a bug introduced in Synapse 1.64.0rc1 which could cause log spam when fetching events from other homeservers. ([\matrix-org#14347](matrix-org#14347))
- Fix a bug introduced in 1.66 which would not send certain pushrules to clients. Contributed by Nico. ([\matrix-org#14356](matrix-org#14356))
- Fix a bug introduced in v1.71.0rc1 where the power level event was incorrectly created during initial room creation. ([\matrix-org#14361](matrix-org#14361))
- Fix the refresh token endpoint to be under /r0 and /v3 instead of /v1. Contributed by Tulir @ Beeper. ([\matrix-org#14364](matrix-org#14364))
- Fix a long-standing bug where Synapse would raise an error when encountering an unrecognised field in a `/sync` filter, instead of ignoring it for forward compatibility. ([\matrix-org#14369](matrix-org#14369))
- Fix a background database update, introduced in Synapse 1.64.0, which could cause poor database performance. ([\matrix-org#14374](matrix-org#14374))
- Fix PostgreSQL sometimes using table scans for queries against the `event_search` table, taking a long time and a large amount of IO. ([\matrix-org#14409](matrix-org#14409))
- Fix rendering of some HTML templates (including emails). Introduced in v1.71.0. ([\matrix-org#14448](matrix-org#14448))
- Fix a bug introduced in Synapse 1.70.0 where the background updates to add non-thread unique indexes on receipts could fail when upgrading from 1.67.0 or earlier. ([\matrix-org#14453](matrix-org#14453))

Updates to the Docker image
---------------------------

- Add all Stream Writer worker types to `configure_workers_and_start.py`. ([\matrix-org#14197](matrix-org#14197))
- Remove references to legacy worker types in the multi-worker Dockerfile. ([\matrix-org#14294](matrix-org#14294))

Improved Documentation
----------------------

- Upload documentation PRs to Netlify. ([\matrix-org#12947](matrix-org#12947), [\matrix-org#14370](matrix-org#14370))
- Add addtional TURN server configuration example based on [eturnal](https://github.com/processone/eturnal) and adjust general TURN server doc structure. ([\matrix-org#14293](matrix-org#14293))
- Add example on how to load balance /sync requests. Contributed by [aceArt](https://aceart.de). ([\matrix-org#14297](matrix-org#14297))
- Edit sample Nginx reverse proxy configuration to use HTTP/1.1. Contributed by Brad Jones. ([\matrix-org#14414](matrix-org#14414))

Deprecations and Removals
-------------------------

- Remove support for PostgreSQL 10. ([\matrix-org#14392](matrix-org#14392), [\matrix-org#14397](matrix-org#14397))

Internal Changes
----------------

- Run unit tests against Python 3.11. ([\matrix-org#13812](matrix-org#13812))
- Add TLS support for generic worker endpoints. ([\matrix-org#14128](matrix-org#14128), [\matrix-org#14455](matrix-org#14455))
- Switch to a maintained action for installing Rust in CI. ([\matrix-org#14313](matrix-org#14313))
- Add override ability to `complement.sh` command line script to request certain types of workers. ([\matrix-org#14324](matrix-org#14324))
- Enabling testing of [MSC3874](matrix-org/matrix-spec-proposals#3874) (filtering of `/messages` by relation type) in complement. ([\matrix-org#14339](matrix-org#14339))
- Concisely log a failure to resolve state due to missing `prev_events`. ([\matrix-org#14346](matrix-org#14346))
- Use a maintained Github action to install Rust. ([\matrix-org#14351](matrix-org#14351))
- Cleanup old worker datastore classes. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#14375](matrix-org#14375))
- Test against PostgreSQL 15 in CI. ([\matrix-org#14394](matrix-org#14394))
- Remove unreachable code. ([\matrix-org#14410](matrix-org#14410))
- Clean-up event persistence code. ([\matrix-org#14411](matrix-org#14411))
- Update docstring to clarify that `get_partial_state_events_batch` does not just give you completely arbitrary partial-state events. ([\matrix-org#14417](matrix-org#14417))
- Fix mypy errors introduced by bumping the locked version of `attrs` and `gitpython`. ([\matrix-org#14433](matrix-org#14433))
- Make Dependabot only bump Rust deps in the lock file. ([\matrix-org#14434](matrix-org#14434))
- Fix an incorrect stub return type for `PushRuleEvaluator.run`. ([\matrix-org#14451](matrix-org#14451))
- Improve performance of `/context` in large rooms. ([\matrix-org#14461](matrix-org#14461))
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.

3 participants