Skip to content

Commit

Permalink
ci: use ruff for linting and formatting (#466)
Browse files Browse the repository at this point in the history
* use `ruff` in pre-commit hooks

* remove unneeded empty line

* use `ruff` everywhere!

* format with `ruff`

* use popular rule selection for linting

* fix warnings reported by `ruff`

* update contributing guide with `ruff`

* fix issues reported by `ruff`

* fix syntax

* use type hints compatible with 3.8+
  • Loading branch information
lvaylet authored May 16, 2024
1 parent dc93b05 commit e1b8610
Show file tree
Hide file tree
Showing 30 changed files with 143 additions and 130 deletions.
1 change: 0 additions & 1 deletion .github/workflows/build-and-push-to-ghcr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ jobs:
permissions:
contents: read
packages: write
#
steps:
- name: Checkout repository
uses: actions/checkout@v4
Expand Down
35 changes: 8 additions & 27 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,36 +8,17 @@
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
rev: v4.6.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-added-large-files
- repo: https://github.com/myint/autoflake
rev: v1.7.6
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.4.4
hooks:
- id: autoflake
args:
- --in-place
- --remove-unused-variables
- --remove-all-unused-imports
# The configuration of flake8, black, isort and pylint is automatically loaded
# from either pyproject.toml or setup.cfg to remain consistent with the Makefile
# targets.
- repo: https://github.com/PyCQA/flake8
rev: 5.0.4
hooks:
- id: flake8
- repo: https://github.com/psf/black
rev: 22.10.0
hooks:
- id: black
- repo: https://github.com/PyCQA/isort
rev: 5.12.0
hooks:
- id: isort
- repo: https://github.com/PyCQA/pylint
rev: v2.15.4
hooks:
- id: pylint
# Run the linter.
- id: ruff
# Run the formatter.
- id: ruff-format
48 changes: 34 additions & 14 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,31 @@ This project follows [Google's Open Source Community Guidelines](https://opensou
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
autoflake................................................................Passed
flake8...................................................................Passed
black....................................................................Passed
isort....................................................................Passed
pylint...................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
```

If any error is reported, your commit gets canceled and you can start working on fixing the issues. If a formatting error gets reported, run `make format` to automatically organize the imports and reformat the code with [`isort`](https://github.com/PyCQA/isort) and [`black`](https://github.com/psf/black). Also make sure to fix any errors returned by linters or static analyzers such as [`flake8`](https://flake8.pycqa.org/en/latest/), [`pylint`](https://pylint.pycqa.org/en/latest/), [`mypy`](http://mypy-lang.org/) or [`pytype`](https://github.com/google/pytype). Then try `git commit`-ting your changes again.
Errors (and/or skipped files) show up like:

```sh
$ pre-commit run
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
ruff.....................................................................Failed
- hook id: ruff
- exit code: 1
slo_generator/backends/prometheus.py:194:57: B006 Do not use mutable data structures for argument defaults
slo_generator/backends/prometheus.py:194:86: B006 Do not use mutable data structures for argument defaults
Found 2 errors.
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).
ruff-format..............................................................Passed
```

If any error is reported, your commit gets canceled and you can start working on fixing the issues. If a formatting error gets reported, run `make format` to automatically organize the imports and reformat the code with [`Ruff`](https://docs.astral.sh/ruff/). Also make sure to fix any errors returned by linters or static analyzers such as [`Ruff`](https://docs.astral.sh/ruff/) (again!), [`mypy`](http://mypy-lang.org/) or [`pytype`](https://github.com/google/pytype). Then try `git commit`-ting your changes again.

Ignoring these pre-commit warnings is not recommended. The Continuous Integration (CI) pipelines will run the exact same checks (and more!) when your commits get pushed. The checks will fail there and prevent you from merging your changes to the `master` branch anyway. So fail fast, fail early, fail often and fix as many errors as possible on your local development machine. Code reviews will be more enjoyable for everyone!

Expand All @@ -66,15 +83,16 @@ This project follows [Google's Open Source Community Guidelines](https://opensou

***Notes:***

- IDEs such as Visual Studio Code and PyCharm can help with linting and reformatting. For example, Visual Studio Code can be configured to run `isort` and `black` automatically on every file save. Just add these lines to your `settings.json` file, as documented in [this article](https://cereblanco.medium.com/setup-black-and-isort-in-vscode-514804590bf9):
- IDEs such as Visual Studio Code and PyCharm can help with linting and reformatting. For example, Visual Studio Code can be configured to run `ruff` automatically on every file save. Just add these lines to your `settings.json` file, as documented on the [official Ruff extension for Visual Studio Code page](https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff):

```json
"editor.formatOnSave": true,
"python.formatting.provider": "black",
"[python]": {
"editor.codeActionsOnSave": {
"source.organizeImports": true // isort
}
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll": "explicit",
"source.organizeImports": "explicit",
},
"editor.defaultFormatter": "charliermarsh.ruff",
},
```

Expand All @@ -90,13 +108,15 @@ You can also select which test to run, and do other things:

```sh
make unit # run unit tests only
make lint # lint code only (with flake8, pylint, mypy and pytype)
make format # format code only (with isort and black)
make lint # lint code only (with ruff, mypy and pytype)
make format # format code only (with ruff)
make docker_test # build Docker image and run tests within Docker container
make docker_build # build Docker image only
make info # see current slo-generator version
```

Inspect `Makefile` for more targets and more details on how they are implemented.

### Adding support for a new backend or exporter

The `slo-generator` tool is designed to be modular as it moves forward. Users, customers and Google folks should be able to easily add the metrics backend or the exporter of their choosing.
Expand Down
20 changes: 5 additions & 15 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,13 @@ coverage:
$(COVERAGE) report --rcfile=".coveragerc"

format:
isort .
black .
ruff format

lint: black isort flake8 pylint pytype mypy
lint: ruff pytype mypy

black:
black . --check

isort:
isort . --check-only

flake8:
flake8 $(NAME)/
flake8 tests/

pylint:
find ./$(NAME) ./tests -type f -name "*.py" | xargs pylint
ruff:
ruff format --check
ruff check

pytype:
pytype
Expand Down
45 changes: 17 additions & 28 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,26 @@
requires = ["setuptools>=42", "wheel"] # PEP 508 specifications.
build-backend = "setuptools.build_meta"

[tool.black]
# Using Black with other tools
# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#using-black-with-other-tools
line-length = 88 # default: 88

[tool.isort]
# Make it compatible with black
profile = "black"
extend_skip = [
".pytype",
]
extend_skip_glob = [
"?venv*", # e.g. venv, .venv, venv3.11, .venv3.11
]

[tool.pylint]
ignore-patterns = [
"test_.*?py",
]

[tool.pylint.messages_control]
max-line-length = 88
disable = [
"logging-fstring-interpolation",
"import-error",
"possibly-used-before-assignment",
]

[tool.mypy]
# https://mypy.readthedocs.io/en/stable/config_file.html#using-a-pyproject-toml-file
ignore_missing_imports = true

[tool.pytype]
inputs = ['slo_generator']

[tool.ruff.lint]
# https://docs.astral.sh/ruff/linter/#rule-selection
select = [
# pycodestyle
"E",
# Pyflakes
"F",
# pyupgrade
"UP",
# flake8-bugbear
"B",
# flake8-simplify
"SIM",
# isort
"I",
]
1 change: 1 addition & 0 deletions samples/custom/custom_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`custom_backend.py`
Dummy sample of a custom backend.
"""

import logging

LOGGER = logging.getLogger(__name__)
Expand Down
1 change: 1 addition & 0 deletions samples/custom/custom_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`custom_exporter.py`
Dummy sample of a custom exporter.
"""

import logging

from slo_generator.exporters.base import MetricsExporter
Expand Down
31 changes: 11 additions & 20 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -102,38 +102,29 @@ pubsub =
cloudevent =
cloudevents
dev =
pip >=23.3 # avoid known vulnerabilities in pip <23.3 (reported by `safety check`)
wheel
flake8
black >=24.3.0 # avoid CVE-2024-21503 (reported by `safety check`)
isort
bandit
GitPython >=3.1.35 # avoid CVE-2023-41040, CVE-2023-40267 and CVE-2023-40590 (reported by `safety check`)
mock
mypy
pip >=23.3 # avoid known vulnerabilities in pip <23.3 (reported by `safety check`)
pre-commit
pytest
pytest-cov
pylint
pytype
mypy
ruff
safety
types-mock
types-PyYAML
types-protobuf
types-python-dateutil
types-setuptools
types-PyYAML
types-requests
types-protobuf
pre-commit
bandit
GitPython >=3.1.35 # avoid CVE-2023-41040, CVE-2023-40267 and CVE-2023-40590 (reported by `safety check`)
safety >=3 # required by `black >=24.3.0`
types-setuptools
wheel

[options.entry_points]
console_scripts =
slo-generator = slo_generator.cli:main

[flake8]
# Why those options?
# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#id1
max-line-length = 88
extend-ignore = E203

# Generated by synthtool. DO NOT EDIT!
[bdist_wheel]
universal = 1
2 changes: 2 additions & 0 deletions slo_generator/backends/cloud_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`cloud_monitoring.py`
Cloud Monitoring backend implementation.
"""

import logging
import pprint
import warnings
Expand Down Expand Up @@ -169,6 +170,7 @@ def exponential_distribution_cut(self, *args, **kwargs):
"exponential_distribution_cut will be deprecated in version 2.0, "
"please use distribution_cut instead",
FutureWarning,
stacklevel=2,
)
return self.distribution_cut(*args, **kwargs)

Expand Down
2 changes: 2 additions & 0 deletions slo_generator/backends/cloud_monitoring_mql.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`cloud_monitoring_mql.py`
Cloud Monitoring backend implementation with MQL (Monitoring Query Language).
"""

import logging
import pprint
import typing
Expand Down Expand Up @@ -170,6 +171,7 @@ def exponential_distribution_cut(self, *args, **kwargs) -> Tuple[int, int]:
"exponential_distribution_cut will be deprecated in version 2.0, "
"please use distribution_cut instead",
FutureWarning,
stacklevel=2,
)
return self.distribution_cut(*args, **kwargs)

Expand Down
2 changes: 2 additions & 0 deletions slo_generator/backends/cloud_service_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`cloud_service_monitoring.py`
Cloud Service Monitoring exporter class.
"""

import difflib
import json
import logging
Expand Down Expand Up @@ -315,6 +316,7 @@ def build_service_id(
"It will be removed in version 3.0, please use MeshIstio "
"instead",
FutureWarning,
stacklevel=2,
)
if "zone" in cluster_istio:
cluster_istio["suffix"] = "zone"
Expand Down
1 change: 1 addition & 0 deletions slo_generator/backends/dynatrace.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`dynatrace.py`
Datadog backend implementation.
"""

import json
import logging
import pprint
Expand Down
19 changes: 15 additions & 4 deletions slo_generator/backends/prometheus.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import logging
import os
import pprint
from typing import Dict, List, Optional, Tuple
from typing import Dict, List, Optional, Tuple, Union

from prometheus_http_client import Prometheus

Expand Down Expand Up @@ -145,8 +145,8 @@ def query(
filter: str,
window: int,
timestamp: Optional[int] = None,
operators: list = [],
labels: dict = {},
operators: Union[list, None] = None,
labels: Union[dict, None] = None,
) -> dict:
"""Query Prometheus server.
Expand All @@ -160,6 +160,10 @@ def query(
Returns:
dict: Response.
"""
if operators is None:
operators = []
if labels is None:
labels = {}
filter = PrometheusBackend._fmt_query(filter, window, operators, labels)
LOGGER.debug(f"Query: {filter}")
response = self.client.query(metric=filter)
Expand Down Expand Up @@ -187,7 +191,10 @@ def count(response: dict) -> float:
@staticmethod
# pylint: disable=dangerous-default-value
def _fmt_query(
query: str, window: int, operators: List[str] = [], labels: Dict[str, str] = {}
query: str,
window: int,
operators: Union[List[str], None] = None,
labels: Union[Dict[str, str], None] = None,
) -> str:
"""Format Prometheus query:
Expand All @@ -208,6 +215,10 @@ def _fmt_query(
Returns:
str: Formatted query.
"""
if operators is None:
operators = []
if labels is None:
labels = {}
query = query.strip()
if "[window" in query:
query = query.replace("[window", f"[{window}s")
Expand Down
1 change: 1 addition & 0 deletions slo_generator/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
`constants.py`
Constants and environment variables used in `slo-generator`.
"""

import os
from typing import Dict, List, Tuple

Expand Down
Loading

0 comments on commit e1b8610

Please sign in to comment.