From 1742f91a34132b4c51be58c1558c0a67863d8621 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 13:34:26 +0200 Subject: [PATCH 1/8] Cleanup pre-commit lint task --- .pre-commit-config.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fd657dc45..39836899e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -46,11 +46,6 @@ repos: - id: isort args: ["--profile", "black", "--filter-files"] - # - repo: https://github.com/pre-commit/mirrors-pylint - # rev: v3.0.0a3 - # hooks: - # - id: pylint - - repo: https://github.com/codespell-project/codespell rev: v2.1.0 hooks: From 0d88d17f7cbc42c2908a03c495f0371999f12954 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 13:45:27 +0200 Subject: [PATCH 2/8] Use setup extra_requires for dev/prod dependencies Also add pylint by default. --- api/setup.py | 12 ++++++++++-- python_apps/airtime-celery/setup.py | 6 ++++++ python_apps/airtime_analyzer/requirements-dev.txt | 5 ----- python_apps/airtime_analyzer/setup.py | 10 ++++++++++ python_apps/api_clients/requirements-dev.txt | 4 ---- python_apps/api_clients/setup.py | 9 +++++++++ python_apps/pypo/setup.py | 6 ++++++ 7 files changed, 41 insertions(+), 11 deletions(-) delete mode 100644 python_apps/airtime_analyzer/requirements-dev.txt delete mode 100644 python_apps/api_clients/requirements-dev.txt diff --git a/api/setup.py b/api/setup.py index 724901cbe..f255a60e7 100644 --- a/api/setup.py +++ b/api/setup.py @@ -22,11 +22,19 @@ setup( scripts=["bin/libretime-api"], install_requires=[ "coreapi", - "Django~=3.0", + "django~=3.0", "djangorestframework", "django-url-filter", "markdown", "model_bakery", - "psycopg2", ], + extras_require={ + "prod": [ + "psycopg2", + ], + "dev": [ + "psycopg2-binary", + "pylint", + ], + }, ) diff --git a/python_apps/airtime-celery/setup.py b/python_apps/airtime-celery/setup.py index ac0e3ced4..c51705e96 100644 --- a/python_apps/airtime-celery/setup.py +++ b/python_apps/airtime-celery/setup.py @@ -23,5 +23,11 @@ setup( "kombu==4.6.10", "configobj", ], + extras_require={ + "prod": [], + "dev": [ + "pylint", + ], + }, zip_safe=False, ) diff --git a/python_apps/airtime_analyzer/requirements-dev.txt b/python_apps/airtime_analyzer/requirements-dev.txt deleted file mode 100644 index e8b5dfdfc..000000000 --- a/python_apps/airtime_analyzer/requirements-dev.txt +++ /dev/null @@ -1,5 +0,0 @@ -distro -pylint -pytest -pytest-cov -pytest-xdist diff --git a/python_apps/airtime_analyzer/setup.py b/python_apps/airtime_analyzer/setup.py index 01d899f7c..87bbe673a 100644 --- a/python_apps/airtime_analyzer/setup.py +++ b/python_apps/airtime_analyzer/setup.py @@ -33,5 +33,15 @@ setup( # If this version is changed, it needs changing in the install script too "pycairo==1.19.1", ], + extras_require={ + "prod": [], + "dev": [ + "distro", + "pylint", + "pytest", + "pytest-cov", + "pytest-xdist", + ], + }, zip_safe=False, ) diff --git a/python_apps/api_clients/requirements-dev.txt b/python_apps/api_clients/requirements-dev.txt deleted file mode 100644 index 2b9579643..000000000 --- a/python_apps/api_clients/requirements-dev.txt +++ /dev/null @@ -1,4 +0,0 @@ -pylint -pytest -pytest-cov -pytest-xdist diff --git a/python_apps/api_clients/setup.py b/python_apps/api_clients/setup.py index af6f5354f..480811043 100644 --- a/python_apps/api_clients/setup.py +++ b/python_apps/api_clients/setup.py @@ -23,5 +23,14 @@ setup( "python-dateutil>=2.7.0", "requests", ], + extras_require={ + "prod": [], + "dev": [ + "pylint", + "pytest", + "pytest-cov", + "pytest-xdist", + ], + }, zip_safe=False, ) diff --git a/python_apps/pypo/setup.py b/python_apps/pypo/setup.py index ddb3b1037..d34252dc5 100644 --- a/python_apps/pypo/setup.py +++ b/python_apps/pypo/setup.py @@ -37,5 +37,11 @@ setup( "pytz", "requests", ], + extras_require={ + "prod": [], + "dev": [ + "pylint", + ], + }, zip_safe=False, ) From ceb40ee216381e9cb515e3907808b7a2e854ba62 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 13:46:47 +0200 Subject: [PATCH 3/8] Use shared python makefile and start linting Always update setuptools/wheel/pip Never fail while linting We use github annotations to impove the code incrementally. --- api/Makefile | 9 +++++++++ python_apps/airtime-celery/Makefile | 9 +++++++++ python_apps/airtime_analyzer/Makefile | 20 ++++++++---------- python_apps/api_clients/Makefile | 25 +++++++++-------------- python_apps/pypo/Makefile | 9 +++++++++ tools/Makefile | 26 +++++++----------------- tools/python.mk | 29 +++++++++++++++++++++++++++ 7 files changed, 81 insertions(+), 46 deletions(-) create mode 100644 api/Makefile create mode 100644 python_apps/airtime-celery/Makefile create mode 100644 python_apps/pypo/Makefile create mode 100644 tools/python.mk diff --git a/api/Makefile b/api/Makefile new file mode 100644 index 000000000..6999c6187 --- /dev/null +++ b/api/Makefile @@ -0,0 +1,9 @@ +all: lint + +include ../tools/python.mk + +PIP_INSTALL := --editable .[dev] +PYLINT_ARG := libretimeapi + +lint: .pylint +clean: .clean diff --git a/python_apps/airtime-celery/Makefile b/python_apps/airtime-celery/Makefile new file mode 100644 index 000000000..135a53091 --- /dev/null +++ b/python_apps/airtime-celery/Makefile @@ -0,0 +1,9 @@ +all: lint + +include ../../tools/python.mk + +PIP_INSTALL := --editable .[dev] +PYLINT_ARG := airtime-celery + +lint: .pylint +clean: .clean diff --git a/python_apps/airtime_analyzer/Makefile b/python_apps/airtime_analyzer/Makefile index 9d4260e07..8ea046b30 100644 --- a/python_apps/airtime_analyzer/Makefile +++ b/python_apps/airtime_analyzer/Makefile @@ -1,19 +1,15 @@ -.PHONY: lint test +all: lint test -SHELL := bash -CPU_CORES := $(shell nproc) +include ../../tools/python.mk -MODULE_APP := airtime_analyzer -MODULE_TESTS := tests +PIP_INSTALL := --editable .[dev] +PYLINT_ARG := airtime_analyzer tests +PYTEST_ARG := --cov=airtime_analyzer tests -lint: - pylint ${MODULE_APP} - pylint ${MODULE_TESTS} +lint: .pylint fixtures: bash tests/fixtures/generate.sh -test: fixtures - pytest -n ${CPU_CORES} --color=yes -v --cov=${MODULE_APP} ${MODULE_TESTS} - -all: lint test +test: fixtures .pytest +clean: .clean diff --git a/python_apps/api_clients/Makefile b/python_apps/api_clients/Makefile index 267fb7986..5da67fcae 100644 --- a/python_apps/api_clients/Makefile +++ b/python_apps/api_clients/Makefile @@ -1,16 +1,11 @@ -.PHONY: lint test - -SHELL := bash -CPU_CORES := $(shell nproc) - -MODULE_APP := api_clients -MODULE_TESTS := tests - -lint: - pylint ${MODULE_APP} - pylint ${MODULE_TESTS} - -test: - pytest -n ${CPU_CORES} --color=yes -v --cov=${MODULE_APP} ${MODULE_TESTS} - all: lint test + +include ../../tools/python.mk + +PIP_INSTALL := --editable .[dev] +PYLINT_ARG := api_clients tests +PYTEST_ARG := --cov=api_clients tests + +lint: .pylint +test: .pytest +clean: .clean diff --git a/python_apps/pypo/Makefile b/python_apps/pypo/Makefile new file mode 100644 index 000000000..bd2ee0a9c --- /dev/null +++ b/python_apps/pypo/Makefile @@ -0,0 +1,9 @@ +all: lint + +include ../../tools/python.mk + +PIP_INSTALL := --editable .[dev] +PYLINT_ARG := liquidsoap pypo + +lint: .pylint +clean: .clean diff --git a/tools/Makefile b/tools/Makefile index 511ba08af..a8cdf94c9 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -1,23 +1,11 @@ -.PHONY: lint test -.ONESHELL: - -SHELL := bash -CPU_CORES := $(shell nproc) - all: lint test -venv: - python3 -m venv venv - source venv/bin/activate - pip install -r requirements-dev.txt +include python.mk -lint: venv - source venv/bin/activate - pylint tools +PIP_INSTALL := -r requirements-dev.txt +PYLINT_ARG := tools +PYTEST_ARG := . -test: venv - source venv/bin/activate - pytest -n ${CPU_CORES} --color=yes -v . - -clean: - rm -Rf venv +lint: .pylint +test: .pytest +clean: .clean diff --git a/tools/python.mk b/tools/python.mk new file mode 100644 index 000000000..46b642ef4 --- /dev/null +++ b/tools/python.mk @@ -0,0 +1,29 @@ +.ONESHELL: + +SHELL := bash +CPU_CORES := $(shell nproc) + +# PIP_INSTALL := --editable .[dev] +# PYLINT_ARG := +# PYTEST_ARG := + +VENV := venv +$(VENV): + python3 -m venv $(VENV) + source $(VENV)/bin/activate + pip install --upgrade pip setuptools wheel + pip install $(PIP_INSTALL) + +.PHONY: .pylint +.pylint: $(VENV) + source $(VENV)/bin/activate + pylint --output-format=colorized $(PYLINT_ARG) || true + +.PHONY: .pytest +.pytest: $(VENV) + source venv/bin/activate + pytest -n $(CPU_CORES) --color=yes -v $(PYTEST_ARG) + +.PHONY: .clean +.clean: + rm -Rf $(VENV) From 3dbc1205d04e883280c83aad97ab0301947aab99 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 14:41:51 +0200 Subject: [PATCH 4/8] Start linting with mypy --- api/Makefile | 3 ++- api/setup.py | 1 + python_apps/airtime-celery/Makefile | 3 ++- python_apps/airtime-celery/setup.py | 1 + python_apps/airtime_analyzer/Makefile | 3 ++- python_apps/airtime_analyzer/setup.py | 1 + python_apps/api_clients/Makefile | 3 ++- python_apps/api_clients/setup.py | 1 + python_apps/pypo/Makefile | 3 ++- python_apps/pypo/setup.py | 1 + tools/Makefile | 3 ++- tools/python.mk | 6 ++++++ tools/requirements-dev.txt | 1 + 13 files changed, 24 insertions(+), 6 deletions(-) diff --git a/api/Makefile b/api/Makefile index 6999c6187..60e3bf54a 100644 --- a/api/Makefile +++ b/api/Makefile @@ -4,6 +4,7 @@ include ../tools/python.mk PIP_INSTALL := --editable .[dev] PYLINT_ARG := libretimeapi +MYPY_ARG := libretimeapi -lint: .pylint +lint: .pylint .mypy clean: .clean diff --git a/api/setup.py b/api/setup.py index f255a60e7..c4a935e1a 100644 --- a/api/setup.py +++ b/api/setup.py @@ -34,6 +34,7 @@ setup( ], "dev": [ "psycopg2-binary", + "mypy", "pylint", ], }, diff --git a/python_apps/airtime-celery/Makefile b/python_apps/airtime-celery/Makefile index 135a53091..b74c3b0e1 100644 --- a/python_apps/airtime-celery/Makefile +++ b/python_apps/airtime-celery/Makefile @@ -4,6 +4,7 @@ include ../../tools/python.mk PIP_INSTALL := --editable .[dev] PYLINT_ARG := airtime-celery +MYPY_ARG := airtime-celery -lint: .pylint +lint: .pylint .mypy clean: .clean diff --git a/python_apps/airtime-celery/setup.py b/python_apps/airtime-celery/setup.py index c51705e96..d7aebdcbe 100644 --- a/python_apps/airtime-celery/setup.py +++ b/python_apps/airtime-celery/setup.py @@ -26,6 +26,7 @@ setup( extras_require={ "prod": [], "dev": [ + "mypy", "pylint", ], }, diff --git a/python_apps/airtime_analyzer/Makefile b/python_apps/airtime_analyzer/Makefile index 8ea046b30..d0c69d657 100644 --- a/python_apps/airtime_analyzer/Makefile +++ b/python_apps/airtime_analyzer/Makefile @@ -4,9 +4,10 @@ include ../../tools/python.mk PIP_INSTALL := --editable .[dev] PYLINT_ARG := airtime_analyzer tests +MYPY_ARG := airtime_analyzer tests PYTEST_ARG := --cov=airtime_analyzer tests -lint: .pylint +lint: .pylint .mypy fixtures: bash tests/fixtures/generate.sh diff --git a/python_apps/airtime_analyzer/setup.py b/python_apps/airtime_analyzer/setup.py index 87bbe673a..d1dd87475 100644 --- a/python_apps/airtime_analyzer/setup.py +++ b/python_apps/airtime_analyzer/setup.py @@ -37,6 +37,7 @@ setup( "prod": [], "dev": [ "distro", + "mypy", "pylint", "pytest", "pytest-cov", diff --git a/python_apps/api_clients/Makefile b/python_apps/api_clients/Makefile index 5da67fcae..28d7d7224 100644 --- a/python_apps/api_clients/Makefile +++ b/python_apps/api_clients/Makefile @@ -4,8 +4,9 @@ include ../../tools/python.mk PIP_INSTALL := --editable .[dev] PYLINT_ARG := api_clients tests +MYPY_ARG := api_clients tests PYTEST_ARG := --cov=api_clients tests -lint: .pylint +lint: .pylint .mypy test: .pytest clean: .clean diff --git a/python_apps/api_clients/setup.py b/python_apps/api_clients/setup.py index 480811043..d0f24290c 100644 --- a/python_apps/api_clients/setup.py +++ b/python_apps/api_clients/setup.py @@ -26,6 +26,7 @@ setup( extras_require={ "prod": [], "dev": [ + "mypy", "pylint", "pytest", "pytest-cov", diff --git a/python_apps/pypo/Makefile b/python_apps/pypo/Makefile index bd2ee0a9c..8f4eefc4c 100644 --- a/python_apps/pypo/Makefile +++ b/python_apps/pypo/Makefile @@ -4,6 +4,7 @@ include ../../tools/python.mk PIP_INSTALL := --editable .[dev] PYLINT_ARG := liquidsoap pypo +MYPY_ARG := liquidsoap pypo -lint: .pylint +lint: .pylint .mypy clean: .clean diff --git a/python_apps/pypo/setup.py b/python_apps/pypo/setup.py index d34252dc5..355143a6f 100644 --- a/python_apps/pypo/setup.py +++ b/python_apps/pypo/setup.py @@ -40,6 +40,7 @@ setup( extras_require={ "prod": [], "dev": [ + "mypy", "pylint", ], }, diff --git a/tools/Makefile b/tools/Makefile index a8cdf94c9..ad8adad3f 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -4,8 +4,9 @@ include python.mk PIP_INSTALL := -r requirements-dev.txt PYLINT_ARG := tools +MYPY_ARG := . PYTEST_ARG := . -lint: .pylint +lint: .pylint .mypy test: .pytest clean: .clean diff --git a/tools/python.mk b/tools/python.mk index 46b642ef4..814cbcf92 100644 --- a/tools/python.mk +++ b/tools/python.mk @@ -5,6 +5,7 @@ CPU_CORES := $(shell nproc) # PIP_INSTALL := --editable .[dev] # PYLINT_ARG := +# MYPY_ARG := # PYTEST_ARG := VENV := venv @@ -19,6 +20,11 @@ $(VENV): source $(VENV)/bin/activate pylint --output-format=colorized $(PYLINT_ARG) || true +.PHONY: .mypy +.mypy: $(VENV) + source $(VENV)/bin/activate + mypy $(MYPY_ARG) || true + .PHONY: .pytest .pytest: $(VENV) source venv/bin/activate diff --git a/tools/requirements-dev.txt b/tools/requirements-dev.txt index 430110048..cda4c0a74 100644 --- a/tools/requirements-dev.txt +++ b/tools/requirements-dev.txt @@ -1,3 +1,4 @@ +mypy pylint pytest pytest-xdist From 3a336375c2c0b7b9e290574cc8ee150cf3ddbc0a Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 13:47:20 +0200 Subject: [PATCH 5/8] Setup python_requires to >=3.6 Fix python_requires format --- api/setup.py | 1 + python_apps/airtime-celery/setup.py | 1 + python_apps/airtime_analyzer/setup.py | 1 + python_apps/api_clients/setup.py | 1 + python_apps/pypo/setup.py | 1 + 5 files changed, 5 insertions(+) diff --git a/api/setup.py b/api/setup.py index c4a935e1a..5f4b77c4b 100644 --- a/api/setup.py +++ b/api/setup.py @@ -20,6 +20,7 @@ setup( packages=find_packages(), include_package_data=True, scripts=["bin/libretime-api"], + python_requires=">=3.6", install_requires=[ "coreapi", "django~=3.0", diff --git a/python_apps/airtime-celery/setup.py b/python_apps/airtime-celery/setup.py index d7aebdcbe..4a9a81741 100644 --- a/python_apps/airtime-celery/setup.py +++ b/python_apps/airtime-celery/setup.py @@ -18,6 +18,7 @@ setup( }, license="MIT", packages=["airtime-celery"], + python_requires=">=3.6", install_requires=[ "celery==4.4.7", "kombu==4.6.10", diff --git a/python_apps/airtime_analyzer/setup.py b/python_apps/airtime_analyzer/setup.py index d1dd87475..82b44e0e4 100644 --- a/python_apps/airtime_analyzer/setup.py +++ b/python_apps/airtime_analyzer/setup.py @@ -23,6 +23,7 @@ setup( "libretime-analyzer=airtime_analyzer.cli:main", ] }, + python_requires=">=3.6", install_requires=[ "mutagen>=1.31.0", "pika>=1.0.0", diff --git a/python_apps/api_clients/setup.py b/python_apps/api_clients/setup.py index d0f24290c..adb6efd3f 100644 --- a/python_apps/api_clients/setup.py +++ b/python_apps/api_clients/setup.py @@ -18,6 +18,7 @@ setup( }, license="AGPLv3", packages=["api_clients"], + python_requires=">=3.6", install_requires=[ "configobj", "python-dateutil>=2.7.0", diff --git a/python_apps/pypo/setup.py b/python_apps/pypo/setup.py index 355143a6f..ebf0c268c 100644 --- a/python_apps/pypo/setup.py +++ b/python_apps/pypo/setup.py @@ -27,6 +27,7 @@ setup( "bin/airtime-liquidsoap", "bin/pyponotify", ], + python_requires=">=3.6", install_requires=[ "amqplib", "configobj", From 00b73a381910d0efaf8a246878e7706fa77a2761 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 15:12:45 +0200 Subject: [PATCH 6/8] Add CI linting job Use a single lint job Run linting inside a container --- .github/workflows/test.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bab069b74..f8181bf31 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -96,6 +96,31 @@ jobs: cd airtime_mvc/tests php ../../vendor/bin/phpunit + # Start lint the code without failing the entire workflow, should be merged + # into 'test' when the entire matrix succeeds. + lint: + runs-on: ubuntu-latest + container: ghcr.io/libretime/libretime-dev:buster + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@v2 + - uses: actions/cache@v2 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/setup.py') }} + restore-keys: | + ${{ runner.os }}-pip- + + - name: Lint + run: | + make -C api lint + make -C python_apps/airtime_analyzer lint + make -C python_apps/airtime-celery lint + make -C python_apps/api_clients lint + make -C python_apps/pypo lint + test: runs-on: ubuntu-latest strategy: From c9322fbcc837bb4b8f9d8975a4648af8b4ab3938 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 15:13:03 +0200 Subject: [PATCH 7/8] Use makefile in test jobs Reorder test job sorting --- .github/workflows/test.yml | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f8181bf31..3c8214d14 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -126,8 +126,8 @@ jobs: strategy: fail-fast: false matrix: - release: [bionic, buster] context: [python_apps/airtime_analyzer, python_apps/api_clients] + release: [bionic, buster] container: ghcr.io/libretime/libretime-dev:${{ matrix.release }} defaults: @@ -140,20 +140,10 @@ jobs: - uses: actions/cache@v2 with: path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ matrix.context }}-${{ hashFiles('**/setup.py', '**/requirements-dev.txt') }} + key: ${{ runner.os }}-pip-${{ matrix.context }}-${{ hashFiles('**/setup.py') }} restore-keys: | ${{ runner.os }}-pip-${{ matrix.context }} - - name: Install dependencies - run: | - python3 -m venv venv && source venv/bin/activate - pip install --upgrade pip setuptools wheel - pip install -r requirements-dev.txt - pip install -e . - working-directory: ${{ matrix.context }} - - name: Test - run: | - source venv/bin/activate - make test + run: make test working-directory: ${{ matrix.context }} From 15145039b3fc8151849e1da431a34e98720975ef Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 10 Sep 2021 14:48:33 +0200 Subject: [PATCH 8/8] Add annotations matchers --- .github/annotations/pylint.json | 32 ++++++++++++++++++++++++++++++++ .github/workflows/test.yml | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 .github/annotations/pylint.json diff --git a/.github/annotations/pylint.json b/.github/annotations/pylint.json new file mode 100644 index 000000000..649e1d09b --- /dev/null +++ b/.github/annotations/pylint.json @@ -0,0 +1,32 @@ +{ + "problemMatcher": [ + { + "severity": "error", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): (E\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-error" + }, + { + "severity": "warning", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): ([A-DF-Z]\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-warning" + } + ] +} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3c8214d14..9b93e28cd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -113,6 +113,10 @@ jobs: restore-keys: | ${{ runner.os }}-pip- + - name: Add annotations matchers + run: | + echo "::add-matcher::.github/annotations/pylint.json" + - name: Lint run: | make -C api lint