From 4e1ed662bff6d2f04dc055f11184a1083a53063c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 21 Apr 2023 16:41:44 +0100 Subject: [PATCH] Modernise tooling and fix CI failing (#335) * Move from setup.py to pyproject.toml Co-authored-by: Patrick Cloke * Ensure unit tests run IDK we sometimes have to invoke trial explicitly like this * Use ruff instead of flake8 And remove py2-style type annotations, which ruff doesn't recognise Co-authored-by: Patrick Cloke * Bump GHA actions part of https://github.com/matrix-org/synapse/issues/14203 * Run black Presumably this fixes complaints from #333 and #334 * Move isort config to pyproject * Also lint stubs * Bump GHA actions part of https://github.com/matrix-org/synapse/issues/14203 * Fix an unintended rename in #333 Needed for unit tests to be happy * Changelog --------- Co-authored-by: Patrick Cloke --- .github/workflows/changelog_check.yml | 4 +- .github/workflows/docker_check.yml | 4 +- .github/workflows/docker_push.yml | 4 +- .github/workflows/pipeline.yml | 27 +++---- changelog.d/335.misc | 1 + pyproject.toml | 80 +++++++++++++++++++++ scripts-dev/lint.sh | 8 +-- scripts-dev/old_deps.py | 57 --------------- setup.cfg | 22 ------ setup.py | 85 ----------------------- stubs/twisted/web/http.pyi | 2 +- sygnal/apnspushkin.py | 1 - sygnal/gcmpushkin.py | 8 ++- sygnal/helper/proxy/proxyagent_twisted.py | 7 +- tests/test_concurrency_limit.py | 16 +++-- tests/testutils.py | 2 +- tox.ini | 8 +-- 17 files changed, 132 insertions(+), 204 deletions(-) create mode 100644 changelog.d/335.misc delete mode 100755 scripts-dev/old_deps.py delete mode 100644 setup.cfg delete mode 100755 setup.py diff --git a/.github/workflows/changelog_check.yml b/.github/workflows/changelog_check.yml index 1ce257c6..0416535a 100644 --- a/.github/workflows/changelog_check.yml +++ b/.github/workflows/changelog_check.yml @@ -6,11 +6,11 @@ jobs: if: ${{ github.base_ref == 'main' || contains(github.base_ref, 'release-') }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: fetch-depth: 0 ref: ${{github.event.pull_request.head.sha}} - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v4 with: python-version: "3.7" - run: python -m pip install towncrier diff --git a/.github/workflows/docker_check.yml b/.github/workflows/docker_check.yml index af04002a..3a422143 100644 --- a/.github/workflows/docker_check.yml +++ b/.github/workflows/docker_check.yml @@ -33,10 +33,10 @@ jobs: # we explicitly check out the repository (and use `context: .` in buildx) # because we need to preserve the git metadata so that setuptools_scm - # (as part of Sygnal's setup.py) can deduce the package version. + # (part of build system config in pyproject.toml) can deduce the package version. # See: https://github.com/marketplace/actions/build-and-push-docker-images#path-context - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Build all platforms uses: docker/build-push-action@v2 diff --git a/.github/workflows/docker_push.yml b/.github/workflows/docker_push.yml index cdec15d1..1d68a467 100644 --- a/.github/workflows/docker_push.yml +++ b/.github/workflows/docker_push.yml @@ -43,10 +43,10 @@ jobs: # we explicitly check out the repository (and use `context: .` in buildx) # because we need to preserve the git metadata so that setuptools_scm - # (as part of Sygnal's setup.py) can deduce the package version. + # (part of build system config in pyproject.toml) can deduce the package version. # See: https://github.com/marketplace/actions/build-and-push-docker-images#path-context - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Build and push all platforms uses: docker/build-push-action@v2 diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 53ab75b9..f747f349 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -8,8 +8,8 @@ jobs: check-code-style: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 with: python-version: "3.7" - run: python -m pip install tox @@ -18,8 +18,8 @@ jobs: check-types-mypy: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 with: python-version: "3.7" - run: python -m pip install tox @@ -30,24 +30,27 @@ jobs: needs: [check-code-style, check-types-mypy] runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 with: python-version: "3.7" - run: python -m pip install -e . - - run: trial tests + - run: python -m twisted.trial tests run-unit-tests-olddeps: name: Unit tests (old dependencies) needs: [ check-code-style, check-types-mypy ] runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 with: python-version: "3.7" - - name: Install old dependencies - run: pip install -r <(./scripts-dev/old_deps.py) + - name: Patch pyproject.toml to require oldest dependencies + run: | + # Ugly. Could use something like https://pyproject-parser.readthedocs.io/en/latest/cli.html#info in the future. + sed --in-place=.bak -e 's/>=/==/g' pyproject.toml + diff pyproject.toml.bak pyproject.toml || true # diff returns 1 if there is a change - name: Install Sygnal run: python -m pip install -e . - - run: trial tests + - run: python -m twisted.trial tests diff --git a/changelog.d/335.misc b/changelog.d/335.misc new file mode 100644 index 00000000..ff4b49e6 --- /dev/null +++ b/changelog.d/335.misc @@ -0,0 +1 @@ +Move from setup.py to pyproject.toml. diff --git a/pyproject.toml b/pyproject.toml index 96aa7878..2ebd7ee1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,3 +33,83 @@ directory = "misc" name = "Internal Changes" showcontent = true + +[tool.isort] +line_length = 88 +sections = "FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,TESTS,LOCALFOLDER" +default_section = "THIRDPARTY" +known_first_party = "sygnal" +known_tests = "tests" +multi_line_output = 3 +include_trailing_comma = true +combine_as_imports = true + + +[tool.ruff] +line-length = 88 +ignore = [ + "E501", # https://beta.ruff.rs/docs/rules/line-too-long/. Black enforces this for us. +] + +[build-system] +build-backend = "setuptools.build_meta" +# Setuptools 64 adds support for PEP 660, which makes `pip install -e .` work. +requires = ["setuptools>=64", "setuptools_scm[toml]>=6.2"] + +[tool.setuptools_scm] +# Note: including this section without any keys still has meaning, see +# https://github.com/pypa/setuptools_scm/blob/51b3566170be25582b5c3216a54b024caf3d431f/README.rst?plain=1#L63-L71 + +[tool.setuptools.packages.find] +exclude = ["tests", "tests.*"] + +[project] +name = "matrix-sygnal" +dynamic = ["version"] +description = "Reference Push Gateway for Matrix Notifications" +readme = {file = "README.md", content-type = "text/markdown"} +requires-python = "~=3.7" +license = {file = "LICENSE"} +authors = [ + {name = "Matrix.org Team and contributors", email = "packages@matrix.org"} +] +dependencies = [ + "aioapns>=1.10,<2.1", + "attrs>=19.2.0", + "cryptography>=2.6.1", + "idna>=2.8", + 'importlib_metadata;python_version<"3.8"', + "jaeger-client>=4.0.0", + "matrix-common==1.0.0", + "opentracing>=2.2.0", + "prometheus_client>=0.7.0,<0.8", + "py-vapid>=1.7.0", + "pyOpenSSL>=17.5.0", + "pywebpush>=1.13.0", + "pyyaml>=5.1.1", + "sentry-sdk>=0.10.2", + "service_identity>=18.1.0", + "Twisted>=19.7", + 'typing-extensions>=3.7.4;python_version<"3.8"', + "zope.interface>=4.6.0", +] + +[project.optional-dependencies] +dev = [ + "black==22.3.0", + "coverage~=5.5", + "ruff==0.0.262", + "isort~=5.0", + "mypy==0.812", + "mypy-zope==0.3.0", + "towncrier", + "tox", + "types-opentracing>=2.4.2", + "typing-extensions>=3.7.4", +] + +[project.urls] +homepage = "https://github.com/matrix-org/sygnal" +documentation = "https://github.com/matrix-org/sygnal/tree/main/docs" +repository = "https://github.com/matrix-org/sygnal.git" +changelog = "https://github.com/matrix-org/sygnal/blob/main/CHANGELOG.md" \ No newline at end of file diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index b3c5532b..77bb5123 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -3,7 +3,8 @@ # Runs linting scripts over the local Sygnal checkout # isort - sorts import statements # black - opinionated code formatter -# flake8 - lints and finds mistakes +# ruff - lints and finds mistakes +# mypy - type checker set -e @@ -84,8 +85,7 @@ else files=( "sygnal" "tests" - "scripts-dev" - "setup.py" + "stubs" ) fi fi @@ -98,5 +98,5 @@ set -x isort "${files[@]}" python3 -m black "${files[@]}" -flake8 "${files[@]}" +ruff --quiet --fix "${files[@]}" mypy "${files[@]}" diff --git a/scripts-dev/old_deps.py b/scripts-dev/old_deps.py deleted file mode 100755 index bdcf29fb..00000000 --- a/scripts-dev/old_deps.py +++ /dev/null @@ -1,57 +0,0 @@ -#!/usr/bin/env python -# -# Copyright 2021 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import os -from typing import Any, Dict, Sequence - -""" -This script outputs a list of requirements, which, if fed to pip install -r, -will install the oldest versions of the dependencies specified by the -setup.py file in the repository root. -""" - - -here = os.path.abspath(os.path.dirname(__file__)) - - -def read_file(path_segments: Sequence[str]) -> str: - """Read a file from the package. - - Params: - path_segments: a list of strings to join to make the path. - """ - file_path = os.path.join(here, *path_segments) - with open(file_path) as f: - return f.read() - - -def exec_file(path_segments: Sequence[str]) -> Dict[str, Any]: - """Execute a single python file to get the variables defined in it. - - Params: - path_segments: a list of strings to join to make the path of the - Python file to execute. - """ - result: Dict[str, Any] = {} - code = read_file(path_segments) - exec(code, result) - return result - - -if __name__ == "__main__": - dependencies = exec_file(("..", "setup.py")) - for requirement in dependencies["INSTALL_REQUIRES"]: - print(requirement.replace(">=", "==")) diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index c13deacb..00000000 --- a/setup.cfg +++ /dev/null @@ -1,22 +0,0 @@ -[flake8] -# line length defaulted to by black -max-line-length = 88 - -# see https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes -# for error codes. The ones we ignore are: -# W503: line break before binary operator -# W504: line break after binary operator -# E203: whitespace before ':' (which is contrary to pep8?) -# (this is a subset of those ignored in Synapse) -ignore=W503,W504,E203 - -[isort] -line_length = 88 -not_skip = __init__.py -sections=FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,TESTS,LOCALFOLDER -default_section=THIRDPARTY -known_first_party=sygnal -known_tests=tests -multi_line_output=3 -include_trailing_comma=true -combine_as_imports=true diff --git a/setup.py b/setup.py deleted file mode 100755 index 2c94bd40..00000000 --- a/setup.py +++ /dev/null @@ -1,85 +0,0 @@ -#!/usr/bin/env python - -# Copyright 2014 OpenMarket Ltd -# Copyright 2017 Vector Creations Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import os.path -from typing import Sequence - -from setuptools import find_packages, setup - -INSTALL_REQUIRES = [ - "aioapns>=1.10,<2.1", - "attrs>=19.2.0", - "cryptography>=2.6.1", - "idna>=2.8", - 'importlib_metadata;python_version<"3.8"', - "jaeger-client>=4.0.0", - "matrix-common==1.0.0", - "opentracing>=2.2.0", - "prometheus_client>=0.7.0,<0.8", - "py-vapid>=1.7.0", - "pyOpenSSL>=17.5.0", - "pywebpush>=1.13.0", - "pyyaml>=5.1.1", - "sentry-sdk>=0.10.2", - "service_identity>=18.1.0", - "Twisted>=19.7", - 'typing-extensions>=3.7.4;python_version<"3.8"', - "zope.interface>=4.6.0", -] - -EXTRAS_REQUIRE = { - "dev": [ - "black==22.3.0", - "coverage~=5.5", - "flake8==3.9.0", - "isort~=5.0", - "mypy==0.812", - "mypy-zope==0.3.0", - "towncrier", - "tox", - "types-opentracing>=2.4.2", - "typing-extensions>=3.7.4", - ] -} - - -def read_file(path_segments: Sequence[str]) -> str: - """Read a file from the package. - - Params: - path_segments: a list of strings to join to make the path. - """ - here = os.path.abspath(os.path.dirname(__file__)) - file_path = os.path.join(here, *path_segments) - with open(file_path) as f: - return f.read() - - -if __name__ == "__main__": - setup( - name="matrix-sygnal", - packages=find_packages(exclude=["tests", "tests.*"]), - description="Reference Push Gateway for Matrix Notifications", - use_scm_version=True, - python_requires=">=3.7", - setup_requires=["setuptools_scm"], - install_requires=INSTALL_REQUIRES, - extras_require=EXTRAS_REQUIRE, - long_description=read_file(("README.md",)), - long_description_content_type="text/markdown", - ) diff --git a/stubs/twisted/web/http.pyi b/stubs/twisted/web/http.pyi index 3a33fa52..a814ed37 100644 --- a/stubs/twisted/web/http.pyi +++ b/stubs/twisted/web/http.pyi @@ -6,7 +6,7 @@ from twisted.internet.interfaces import IAddress, ITCPTransport from twisted.logger import Logger from twisted.protocols.basic import LineReceiver from twisted.web.http_headers import Headers -from twisted.web.iweb import IRequest, IAccessLogFormatter +from twisted.web.iweb import IAccessLogFormatter, IRequest from zope.interface import implementer, provider class HTTPChannel: ... diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py index 8a39cbe7..f3a41cec 100644 --- a/sygnal/apnspushkin.py +++ b/sygnal/apnspushkin.py @@ -246,7 +246,6 @@ async def _dispatch_request( ) try: - with ACTIVE_REQUESTS_GAUGE.track_inprogress(): with SEND_TIME_HISTOGRAM.time(): response = await self._send_notification(request) diff --git a/sygnal/gcmpushkin.py b/sygnal/gcmpushkin.py index 9222270d..7ff63370 100644 --- a/sygnal/gcmpushkin.py +++ b/sygnal/gcmpushkin.py @@ -376,7 +376,13 @@ async def _dispatch_notification_unlimited( else: body["registration_ids"] = pushkeys - log.info("Sending (attempt %i) => %r room:%s, event:%s", retry_number, pushkeys, n.room_id, n.event_id) + log.info( + "Sending (attempt %i) => %r room:%s, event:%s", + retry_number, + pushkeys, + n.room_id, + n.event_id, + ) try: span_tags = {"retry_num": retry_number} diff --git a/sygnal/helper/proxy/proxyagent_twisted.py b/sygnal/helper/proxy/proxyagent_twisted.py index 0867d84f..c7697174 100644 --- a/sygnal/helper/proxy/proxyagent_twisted.py +++ b/sygnal/helper/proxy/proxyagent_twisted.py @@ -80,9 +80,9 @@ def __init__( parsed_url = decompose_http_proxy_url(proxy_url_str) self._proxy_auth = parsed_url.credentials - self.proxy_endpoint = HostnameEndpoint( + self.proxy_endpoint: Optional[HostnameEndpoint] = HostnameEndpoint( reactor, parsed_url.hostname, parsed_url.port, **self._endpoint_kwargs - ) # type: Optional[HostnameEndpoint] + ) else: self.proxy_endpoint = None @@ -124,11 +124,12 @@ def request(self, method, uri, headers=None, bodyProducer=None): pool_key: tuple = (parsed_uri.scheme, parsed_uri.host, parsed_uri.port) request_path = parsed_uri.originForm + endpoint: IStreamClientEndpoint if parsed_uri.scheme == b"http" and self.proxy_endpoint: # Cache *all* connections under the same key, since we are only # connecting to a single destination, the proxy: pool_key = ("http-proxy", self.proxy_endpoint) - endpoint = self.proxy_endpoint # type: IStreamClientEndpoint + endpoint = self.proxy_endpoint request_path = uri elif parsed_uri.scheme == b"https" and self.proxy_endpoint: endpoint = HTTPConnectProxyEndpoint( diff --git a/tests/test_concurrency_limit.py b/tests/test_concurrency_limit.py index b4acf90f..958521f4 100644 --- a/tests/test_concurrency_limit.py +++ b/tests/test_concurrency_limit.py @@ -13,16 +13,18 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Any, Dict, List - -from sygnal.notifications import ConcurrencyLimitedPushkin, Device, Notification +from typing import Any, Dict, List + +from sygnal.notifications import ( + ConcurrencyLimitedPushkin, + Device, + Notification, + NotificationContext, +) from sygnal.utils import twisted_sleep from tests.testutils import TestCase -if TYPE_CHECKING: - from sygnal.notifications import NotificationContext - DEVICE_GCM1_EXAMPLE = { "app_id": "com.example.gcm", "pushkey": "spqrg", @@ -41,7 +43,7 @@ class SlowConcurrencyLimitedDummyPushkin(ConcurrencyLimitedPushkin): - async def dispatch_notification( + async def _dispatch_notification_unlimited( self, n: Notification, device: Device, context: "NotificationContext" ) -> List[str]: """ diff --git a/tests/testutils.py b/tests/testutils.py index 3c08de2c..30f84583 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -337,6 +337,6 @@ def process_request(self, method: bytes, request_path: bytes, content: BinaryIO) """pretend that a request has arrived, and process it""" # this is normally done by HTTPChannel, in its various lineReceived etc methods - req = self.site.requestFactory(self) # type: Request + req: Request = self.site.requestFactory(self) req.content = content req.requestReceived(method, request_path, b"1.1") diff --git a/tox.ini b/tox.ini index 4d9e14e5..4de59129 100644 --- a/tox.ini +++ b/tox.ini @@ -23,11 +23,11 @@ commands = [testenv:check_codestyle] commands = - flake8 sygnal/ tests/ setup.py scripts-dev/ - black --check --diff sygnal/ tests/ setup.py scripts-dev/ - isort --check-only --diff sygnal/ tests/ setup.py scripts-dev/ + ruff sygnal/ tests/ stubs + black --check --diff sygnal/ tests/ stubs + isort --check-only --diff sygnal/ tests/ stubs [testenv:check_types] commands = - mypy sygnal/ tests/ setup.py scripts-dev/ + mypy sygnal/ tests/ stubs