Skip to content

Commit

Permalink
Add Makefile
Browse files Browse the repository at this point in the history
The logic of some of the scripts under "./scripts/" has been moved to
the new Makefile. This improves the developer experience since this
Makefile provides all the commands a developer needs to write code in
this repo.

Apart from that, this commit also fixes the last lint issues, so now the
linter returns a 10/10 score.
  • Loading branch information
jordipiqueselles committed Mar 29, 2024
1 parent 9f47365 commit fc70039
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 68 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/test-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
fetch-depth: 0

- name: Check pyproject.toml
run: ./scripts/check-pyproject.sh
run: make check-pyproject

- name: Set up Python 3.11
uses: actions/setup-python@v4
Expand All @@ -30,7 +30,7 @@ jobs:
run: poetry install

- name: Linter
run: scripts/format-code.sh --check
run: make lint

- name: Run unittests
run: poetry run pytest tests --timeout 15
run: make unnittests
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,6 @@ cython_debug/
# and can be added to the global gitignore or merged into this file. For a more nuclear
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
#.idea/

# Make
out/
45 changes: 45 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
SRC_DIR := ./generic_k8s_webhook
TEST_DIR := ./tests
SRC_FILES := $(shell find $(SRC_DIR) -type f -name '*.py')
TEST_FILES := $(shell find $(TEST_DIR) -type f -name '*.py')

out/install-deps.stamp: pyproject.toml poetry.lock
poetry install
mkdir -p out
touch out/install-deps.stamp

install-deps: out/install-deps.stamp

out/build.stamp: install-deps $(SRC_FILES) $(TEST_FILES)
poetry build
touch out/build.stamp

build: out/build.stamp

.PHONY: lint
lint: build
poetry run isort $(SRC_DIR) $(TEST_DIR) -c
poetry run black $(SRC_DIR) $(TEST_DIR) --check
poetry run pylint $(SRC_DIR) -v

.PHONY: format
format: build
poetry run isort $(SRC_DIR) $(TEST_DIR)
poetry run black $(SRC_DIR) $(TEST_DIR)

.PHONY: unittests
unittests: build
poetry run pytest tests --timeout 15

.PHONY: check-pyproject
check-pyproject:
echo "Check the pyproject.toml has 'version = \"0.0.0\"'"
grep 'version = "0.0.0"' pyproject.toml

.PHONY: docker
docker:
docker build -t generic-k8s-webhook:latest .

all-tests: check-pyproject lint unittests docker

all-tests-seq: | check-pyproject lint unittests docker
22 changes: 13 additions & 9 deletions docs/contributor-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
## Required tools

- [Git](https://git-scm.com/downloads)
- [Python 3.10+](https://www.python.org/downloads/)
- [Python 3.11+](https://www.python.org/downloads/)
- [Poetry](https://python-poetry.org/docs/)
- [Docker](https://docs.docker.com/install/)

Expand All @@ -29,44 +29,48 @@ poetry run python3 generic_k8s_webhook/main.py --config <GenericWebhookConfigFil

## Format code

The project uses [black](https://black.readthedocs.io/en/stable/) and [isort](https://pycqa.github.io/isor) to format the code. To do so automatically, you can run this script:
The project uses [black](https://black.readthedocs.io/en/stable/) and [isort](https://pycqa.github.io/isor) to format the code. To do so automatically, you can execute this make rule:

```bash
./scripts/format-code.sh
make format
```

## Test

You can run all the tests executed in the CI by calling:
You can run (sequentially) all the tests executed in the CI by calling:

```bash
./scripts/run-all-tests.sh
make all-tests-seq
```

These tests can be splitted in 3 categories.

### Linter

The linting phase checks that the code is well [formatted](#format-code). Apart from that, it also runs [pylint](https://www.pylint.org/). In order to pass the linting phase, `pylint` must not detect any error and must give an overall score of >9.5.
The linting phase checks that the code is well [formatted](#format-code). Apart from that, it also runs [pylint](https://www.pylint.org/). In order to pass the linting phase, `pylint` must not detect any error,
even if it's a minor one.

```bash
./scripts/format-code.sh --check
make lint
```

In some cases, it's acceptable to add `# pylint: disable=<rule>` to disable
a specific linting issue in a specific line, only if fixing this issue makes the code worse.

### Unittests and e2e tests

We use [pytest](https://docs.pytest.org/en/7.3.x/) to test the functionality of the app. These tests are defined under the [tests](../tests/) directory and they are a mix of both pure unittests and end-to-end tests.

```bash
poetry run pytest tests
make unittests
```

### Docker build

The last phase of our testing suite is building the docker container that has our app installed in it.

```bash
docker build -t generic-k8s-webhook:latest .
make docker
```

## Structure of the code
Expand Down
2 changes: 1 addition & 1 deletion generic_k8s_webhook/config_parser/entrypoint.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import copy

import generic_k8s_webhook.config_parser.expr_parser as expr_parser
import generic_k8s_webhook.config_parser.operator_parser as op_parser
from generic_k8s_webhook import utils
from generic_k8s_webhook.config_parser import expr_parser
from generic_k8s_webhook.config_parser.action_parser import ActionParserV1
from generic_k8s_webhook.config_parser.jsonpatch_parser import JsonPatchParserV1
from generic_k8s_webhook.config_parser.webhook_parser import WebhookParserV1
Expand Down
2 changes: 1 addition & 1 deletion generic_k8s_webhook/config_parser/expr_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def ref(self, items):

def boolean(self, items):
(elem,) = items
elem_bool = True if elem == "true" else False
elem_bool = elem == "true"
return op.Const(elem_bool)


Expand Down
2 changes: 1 addition & 1 deletion generic_k8s_webhook/config_parser/operator_parser.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import abc
import inspect

import generic_k8s_webhook.config_parser.expr_parser as expr_parser
from generic_k8s_webhook import operators, utils
from generic_k8s_webhook.config_parser import expr_parser
from generic_k8s_webhook.config_parser.common import ParsingException


Expand Down
8 changes: 4 additions & 4 deletions generic_k8s_webhook/http_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def run(self) -> None:
while not self.stop_event.wait(self.refresh_period):
try:
self._reload_manifest()
except Exception as e:
except Exception as e: # pylint: disable=broad-exception-caught
logging.error(e, exc_info=True)

def get_webhooks(self) -> list[Webhook]:
Expand All @@ -61,7 +61,7 @@ class BaseHandler(http.server.BaseHTTPRequestHandler):
def do_GET(self):
try:
self._do_get()
except Exception as e:
except Exception as e: # pylint: disable=broad-exception-caught
logging.error(e, exc_info=True)

def _do_get(self):
Expand All @@ -74,7 +74,7 @@ def _do_get(self):
def do_POST(self):
try:
self._do_post()
except Exception as e:
except Exception as e: # pylint: disable=broad-exception-caught
logging.error(e, exc_info=True)

def _do_post(self):
Expand Down Expand Up @@ -124,7 +124,7 @@ def _get_path(self) -> str:


class Server:
def __init__(
def __init__( # pylint: disable=too-many-arguments
self, port: int, certfile: str, keyfile: str, generic_webhook_config_file: str, config_refresh_period: float = 5
) -> None:
"""Validating/Mutating webhook server. It listens to requests made at port <port>
Expand Down
2 changes: 1 addition & 1 deletion generic_k8s_webhook/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def cli(args):
def start_server(args):
server = Server(args.port, args.cert_file, args.key_file, args.config)

def stop_server(*args):
def stop_server(*args): # pylint: disable=unused-argument
threading.Thread(target=server.stop).start()

signal.signal(signal.SIGINT, stop_server)
Expand Down
5 changes: 2 additions & 3 deletions generic_k8s_webhook/operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,9 @@ def get_value(self, contexts: list) -> Any:
list_arg_values = self.args.get_value(contexts)
if len(list_arg_values) < 2:
return True
elif len(list_arg_values) == 2:
if len(list_arg_values) == 2:
return self._op(list_arg_values[0], list_arg_values[1])
else:
raise ValueError("A comparison cannot have more than 2 operands")
raise ValueError("A comparison cannot have more than 2 operands")

def input_type(self) -> type | None:
return list[None]
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ missing-module-docstring, \
missing-class-docstring, \
missing-function-docstring, \
too-few-public-methods, \
fixme, \
"""

[build-system]
Expand Down
9 changes: 0 additions & 9 deletions scripts/check-pyproject.sh

This file was deleted.

21 changes: 0 additions & 21 deletions scripts/format-code.sh

This file was deleted.

15 changes: 0 additions & 15 deletions scripts/run-all-tests.sh

This file was deleted.

0 comments on commit fc70039

Please sign in to comment.