diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 0afc4c9cd..b03f49f57 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -84,6 +84,7 @@ jobs: path: | .lint.json include-hidden-files: true + overwrite: false lint-typing: name: Lint Typing (Python-${{ matrix.python-versions }}) @@ -144,6 +145,7 @@ jobs: name: security-python${{ matrix.python-versions }} path: .security.json include-hidden-files: true + overwrite: false check-format: name: Check Format diff --git a/.github/workflows/fast-tests.yml b/.github/workflows/fast-tests.yml index 28a5a6954..44510088d 100644 --- a/.github/workflows/fast-tests.yml +++ b/.github/workflows/fast-tests.yml @@ -38,6 +38,7 @@ jobs: name: coverage-python${{ matrix.python-versions }}-fast path: .coverage include-hidden-files: true + overwrite: false fast-tests-extension: uses: ./.github/workflows/fast-tests-extension.yml diff --git a/.github/workflows/slow-checks.yml b/.github/workflows/slow-checks.yml index 46ed19863..d04176fc1 100644 --- a/.github/workflows/slow-checks.yml +++ b/.github/workflows/slow-checks.yml @@ -6,7 +6,7 @@ on: jobs: build-matrix: name: Build Matrix - uses: ./.github/workflows/matrix-all.yml + uses: ./.github/workflows/matrix-python.yml permissions: contents: read @@ -37,10 +37,12 @@ jobs: - name: Run Integration Tests id: run-integration-tests run: poetry run -- nox -s test:integration -- --coverage + - name: Upload Artifacts id: upload-artifacts uses: actions/upload-artifact@v7 with: - name: coverage-python${{ matrix.python-version }}-exasol${{ matrix.exasol-version }}-slow + name: coverage-python${{ matrix.python-version }}-slow path: .coverage include-hidden-files: true + overwrite: false diff --git a/.workflow-patcher.yml b/.workflow-patcher.yml index 270c0dff2..20418f095 100644 --- a/.workflow-patcher.yml +++ b/.workflow-patcher.yml @@ -11,13 +11,3 @@ workflows: uses: actions/checkout@v6 with: fetch-depth: 0 - - name: slow-checks - step_customizations: - - action: REPLACE - job: run-integration-tests - step_id: run-integration-tests - content: - # The PTB integration tests do not need an Exasol Database - - name: Run Integration Tests - id: run-integration-tests - run: poetry run -- nox -s test:integration -- --coverage diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index fe0065a03..5e5bc6c12 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -16,6 +16,7 @@ it only executes `gh-pages.yml`. * Workflow extensions were added to `fast-tests` and `merge-gate`. This allows users to add custom `fast-tests-extension.yml` and `merge-gate-extension.yml` files. For more details, check out the [Workflow Extensions](https://exasol.github.io/python-toolbox/main/user_guide/features/github_workflows/index.html#workflow-extensions) section. +* `slow-checks.yml` is only maintained by the project (not the PTB). See the [Not Maintained by the PTB](https://exasol.github.io/python-toolbox/main/user_guide/features/github_workflows/index.html#not-maintained-by-the-ptb) section. ## Features @@ -27,6 +28,7 @@ details, check out the [Workflow Extensions](https://exasol.github.io/python-too * #730: Added workflow extensions to `fast-tests` and `merge-gate` * #756: Added `dependency-update.yml` to automate resolving vulnerabilities with a generated pull request * #792: Improved `dependency-update.yml` documentation +* #831: Switched `slow-checks.yml` to be provided by the project and not maintained by the PTB and improved output of pydantic validation of `.workflow-patcher.yml` ## Bugfix diff --git a/doc/user_guide/features/github_workflows/index.rst b/doc/user_guide/features/github_workflows/index.rst index c763d1925..10f7f5b8c 100644 --- a/doc/user_guide/features/github_workflows/index.rst +++ b/doc/user_guide/features/github_workflows/index.rst @@ -33,8 +33,9 @@ workflows from the templates. Workflows --------- -The PTB has two categories of workflows: +The PTB has three categories of workflows: #. those maintained by the PTB, which can be modified using the :ref:`workflow_patcher`. + #. those which are seeded by the PTB but owned and maintained by the project after initial creation. #. those which extend the PTB-provided workflows and are maintained by the project (not the PTB). Maintained by the PTB @@ -105,6 +106,21 @@ Maintained by the PTB - Downloads results from code coverage analysis and linting, creates a summary displayed by GitHub as result of running the action, and uploads the results to Sonar. + + +Not Maintained by the PTB +^^^^^^^^^^^^^^^^^^^^^^^^^ + +The PTB seeds these workflows for new projects, but after that the project owns +them and PTB regeneration does not overwrite them. + +.. list-table:: + :widths: 25 25 50 + :header-rows: 1 + + * - Filename + - Run on + - Description * - ``slow-checks.yml`` - Workflow call - Runs long-running checks, which typically involve an Exasol database instance. diff --git a/doc/user_guide/getting_started.rst b/doc/user_guide/getting_started.rst index d7dd2d5a2..1d68bdc9c 100644 --- a/doc/user_guide/getting_started.rst +++ b/doc/user_guide/getting_started.rst @@ -16,7 +16,7 @@ Creating a New Project with Exasol-Toolbox Support .. important:: To establish a new project with toolbox support, you need to have - `Cookiecutter `_ installed: + `Cookiecutter `_ installed: :code:`pip install cookiecutter` diff --git a/exasol/toolbox/templates/github/workflows/checks.yml b/exasol/toolbox/templates/github/workflows/checks.yml index be669a16a..6bcb8e1ba 100644 --- a/exasol/toolbox/templates/github/workflows/checks.yml +++ b/exasol/toolbox/templates/github/workflows/checks.yml @@ -84,6 +84,7 @@ jobs: path: | .lint.json include-hidden-files: true + overwrite: false lint-typing: name: Lint Typing (Python-${{ matrix.python-versions }}) @@ -144,6 +145,7 @@ jobs: name: security-python${{ matrix.python-versions }} path: .security.json include-hidden-files: true + overwrite: false check-format: name: Check Format diff --git a/exasol/toolbox/templates/github/workflows/fast-tests.yml b/exasol/toolbox/templates/github/workflows/fast-tests.yml index 24386e17b..9d444371e 100644 --- a/exasol/toolbox/templates/github/workflows/fast-tests.yml +++ b/exasol/toolbox/templates/github/workflows/fast-tests.yml @@ -37,6 +37,7 @@ jobs: name: coverage-python${{ matrix.python-versions }}-fast path: .coverage include-hidden-files: true + overwrite: false (% if workflow_extension.fast_tests %) fast-tests-extension: diff --git a/exasol/toolbox/templates/github/workflows/slow-checks.yml b/exasol/toolbox/templates/github/workflows/slow-checks.yml index d94cf01ad..69d48cb2a 100644 --- a/exasol/toolbox/templates/github/workflows/slow-checks.yml +++ b/exasol/toolbox/templates/github/workflows/slow-checks.yml @@ -1,3 +1,5 @@ +# This workflow is seeded by the PTB for new projects, but after creation it is +# owned and maintained by the project. name: Slow-Checks on: @@ -45,3 +47,4 @@ jobs: name: coverage-python${{ matrix.python-version }}-exasol${{ matrix.exasol-version }}-slow path: .coverage include-hidden-files: true + overwrite: false diff --git a/exasol/toolbox/util/workflows/exceptions.py b/exasol/toolbox/util/workflows/exceptions.py index bdaaa5ac7..b66acd3e6 100644 --- a/exasol/toolbox/util/workflows/exceptions.py +++ b/exasol/toolbox/util/workflows/exceptions.py @@ -1,5 +1,8 @@ from collections.abc import Mapping from pathlib import Path +from pprint import pformat + +from pydantic import ValidationError class YamlError(Exception): @@ -54,7 +57,23 @@ class InvalidWorkflowPatcherYamlError(YamlError): :class:`WorkflowPatcherConfig`. """ - message_template = "File '{file_path}' is malformed; it failed Pydantic validation." + message_template = ( + "File '{file_path}' is malformed; " + "it failed Pydantic validation with {error_count} errors.\n" + "Validation issue information:\n" + "{validation_details}" + ) + + def __init__(self, file_path: Path, validation_error: ValidationError): + validation_details = ( + f"\033[31m{pformat(validation_error.errors(), sort_dicts=False)}\033[0m" + ) + self.validation_error = validation_error + super().__init__( + file_path=file_path, + error_count=validation_error.error_count(), + validation_details=validation_details, + ) class InvalidWorkflowPatcherEntryError(YamlError): @@ -69,6 +88,29 @@ class InvalidWorkflowPatcherEntryError(YamlError): ) +class InvalidWorkflowNameError(ValueError): + """ + Raised when a workflow name is not one of the available PTB templates. + """ + + def __init__(self, workflow_name: str, valid_workflows): + super().__init__( + f"Invalid workflow: {workflow_name}. Must be one of {valid_workflows}" + ) + + +class NotMaintainedWorkflowError(ValueError): + """ + Raised when a PTB-seeded workflow is requested in an existing project. + """ + + def __init__(self, workflow_name: str): + super().__init__( + f"Workflow '{workflow_name}' is a PTB-seeded workflow that is " + "originally provided by the PTB and can only be seeded for a new project." + ) + + class YamlKeyError(Exception): """ Base exception for when a specified value cannot be found in a YAML. diff --git a/exasol/toolbox/util/workflows/patch_workflow.py b/exasol/toolbox/util/workflows/patch_workflow.py index 322efeeec..d67b383aa 100644 --- a/exasol/toolbox/util/workflows/patch_workflow.py +++ b/exasol/toolbox/util/workflows/patch_workflow.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from enum import Enum from functools import cached_property from typing import ( @@ -19,7 +20,7 @@ from exasol.toolbox.util.workflows import logger from exasol.toolbox.util.workflows.exceptions import InvalidWorkflowPatcherYamlError from exasol.toolbox.util.workflows.render_yaml import YamlRenderer -from exasol.toolbox.util.workflows.templates import WORKFLOW_TEMPLATE_OPTIONS +from exasol.toolbox.util.workflows.templates import validate_workflow_name class ActionType(str, Enum): @@ -69,18 +70,6 @@ class StepCustomization(BaseModel): content: list[StepContent] -def validate_workflow_name(workflow_name: str) -> str: - """ - Validates that the given ``workflow_name`` is a valid workflow name provided by - the PTB. - """ - if workflow_name not in WORKFLOW_TEMPLATE_OPTIONS.keys(): - raise ValueError( - f"Invalid workflow: {workflow_name}. Must be one of {WORKFLOW_TEMPLATE_OPTIONS.keys()}" - ) - return workflow_name - - WorkflowName = Annotated[str, AfterValidator(validate_workflow_name)] @@ -115,6 +104,7 @@ class WorkflowPatcherConfig(BaseModel): ] +@dataclass(frozen=True) class WorkflowPatcher(YamlRenderer): """ The :class:`WorkflowPatcher` enables users to define a YAML file @@ -137,7 +127,9 @@ def content(self) -> CommentedMap: WorkflowPatcherConfig.model_validate(loaded_yaml) return loaded_yaml except ValidationError as ex: - raise InvalidWorkflowPatcherYamlError(file_path=self.file_path) from ex + raise InvalidWorkflowPatcherYamlError( + file_path=self.file_path, validation_error=ex + ) from ex def extract_by_workflow(self, workflow_name: str) -> WorkflowCommentedMap | None: """ diff --git a/exasol/toolbox/util/workflows/templates.py b/exasol/toolbox/util/workflows/templates.py index ef8fef371..8811fc9e6 100644 --- a/exasol/toolbox/util/workflows/templates.py +++ b/exasol/toolbox/util/workflows/templates.py @@ -3,7 +3,13 @@ import importlib_resources as resources +from exasol.toolbox.util.workflows.exceptions import ( + InvalidWorkflowNameError, + NotMaintainedWorkflowError, +) + WORKFLOW_TEMPLATES_DIRECTORY = "exasol.toolbox.templates.github.workflows" +NOT_MAINTAINED_WORKFLOW_NAMES: list[str] = ["slow-checks"] def get_workflow_templates() -> Mapping[str, Path]: @@ -19,4 +25,15 @@ def get_workflow_templates() -> Mapping[str, Path]: } +def validate_workflow_name(workflow_name: str) -> str: + """ + Validate that the given workflow exists and is allowed in the current context. + """ + if workflow_name not in WORKFLOW_TEMPLATE_OPTIONS: + raise InvalidWorkflowNameError(workflow_name, WORKFLOW_TEMPLATE_OPTIONS.keys()) + if workflow_name in NOT_MAINTAINED_WORKFLOW_NAMES: + raise NotMaintainedWorkflowError(workflow_name) + return workflow_name + + WORKFLOW_TEMPLATE_OPTIONS = get_workflow_templates() diff --git a/exasol/toolbox/util/workflows/workflow.py b/exasol/toolbox/util/workflows/workflow.py index 92dfdef06..086414f7d 100644 --- a/exasol/toolbox/util/workflows/workflow.py +++ b/exasol/toolbox/util/workflows/workflow.py @@ -18,6 +18,7 @@ from exasol.toolbox.util.workflows import logger from exasol.toolbox.util.workflows.exceptions import ( InvalidWorkflowPatcherEntryError, + NotMaintainedWorkflowError, YamlError, YamlKeyError, ) @@ -26,7 +27,10 @@ WorkflowPatcher, ) from exasol.toolbox.util.workflows.process_template import WorkflowRenderer -from exasol.toolbox.util.workflows.templates import WORKFLOW_TEMPLATE_OPTIONS +from exasol.toolbox.util.workflows.templates import ( + WORKFLOW_TEMPLATE_OPTIONS, + validate_workflow_name, +) ALL: Final[str] = "all" WORKFLOW_CHOICES: Final[list[str]] = [ALL, *WORKFLOW_TEMPLATE_OPTIONS.keys()] @@ -95,6 +99,7 @@ def update_workflow(workflow_choice: WorkflowChoice, config: BaseConfig) -> None file_path=config.github_workflow_patcher_yaml, ) + is_new_project = not any(config.github_workflow_directory.glob("*.yml")) for workflow_name in workflow_dict: patch_yaml = None if workflow_patcher: @@ -102,6 +107,16 @@ def update_workflow(workflow_choice: WorkflowChoice, config: BaseConfig) -> None workflow_name=workflow_name ) + try: + validate_workflow_name(workflow_name) + except NotMaintainedWorkflowError: + if not is_new_project: + logger.debug( + "Skipping not-maintained workflow in older project: %s", + workflow_name, + ) + continue + try: workflow = Workflow.load_from_template( file_path=workflow_dict[workflow_name], diff --git a/test/unit/util/workflows/patch_workflow_test.py b/test/unit/util/workflows/patch_workflow_test.py index e64179151..e5b69618e 100644 --- a/test/unit/util/workflows/patch_workflow_test.py +++ b/test/unit/util/workflows/patch_workflow_test.py @@ -1,15 +1,20 @@ from inspect import cleandoc from pathlib import Path +from pprint import pformat import pytest from pydantic import ValidationError from ruamel.yaml import CommentedMap -from exasol.toolbox.util.workflows.exceptions import InvalidWorkflowPatcherYamlError +from exasol.toolbox.util.workflows.exceptions import ( + InvalidWorkflowPatcherYamlError, + NotMaintainedWorkflowError, +) from exasol.toolbox.util.workflows.patch_workflow import ( ActionType, WorkflowPatcher, ) +from exasol.toolbox.util.workflows.templates import NOT_MAINTAINED_WORKFLOW_NAMES @pytest.fixture @@ -98,8 +103,15 @@ def test_raises_error_for_unknown_action( ) as ex: workflow_patcher.content + message = ex.value.args[0] + validation_details = message.split("Validation issue information:\n", 1)[1] underlying_error = ex.value.__cause__ assert isinstance(underlying_error, ValidationError) + assert "Input should be 'INSERT_AFTER' or 'REPLACE'" in validation_details + expected_validation_details = ( + f"\033[31m{pformat(underlying_error.errors(), sort_dicts=False)}\033[0m" + ) + assert validation_details == expected_validation_details assert "Input should be 'INSERT_AFTER' or 'REPLACE'" in str(underlying_error) @@ -127,3 +139,30 @@ def test_raises_error_for_unknown_workflow_name( assert "Invalid workflow: unknown-workflow. Must be one of dict_keys([" in str( underlying_error ) + + @staticmethod + def test_rejects_not_maintained_workflow_name( + workflow_patcher_yaml, workflow_patcher + ): + content = f""" + workflows: + - name: "{NOT_MAINTAINED_WORKFLOW_NAMES[0]}" + remove_jobs: + - build-documentation-and-check-links + """ + workflow_patcher_yaml.write_text(cleandoc(content)) + + with pytest.raises( + InvalidWorkflowPatcherYamlError, + match="is malformed; it failed Pydantic validation", + ) as ex: + workflow_patcher.content + + underlying_error = ex.value.__cause__ + assert isinstance(underlying_error, ValidationError) + assert "workflows.0.name" in str(underlying_error) + assert "PTB-seeded workflow" in str(underlying_error) + assert isinstance( + ex.value.validation_error.errors()[0]["ctx"]["error"], + NotMaintainedWorkflowError, + ) diff --git a/test/unit/util/workflows/templates_test.py b/test/unit/util/workflows/templates_test.py index 593aee73d..32a62957a 100644 --- a/test/unit/util/workflows/templates_test.py +++ b/test/unit/util/workflows/templates_test.py @@ -1,4 +1,15 @@ -from exasol.toolbox.util.workflows.templates import get_workflow_templates +import pytest + +from exasol.toolbox.util.workflows.exceptions import ( + InvalidWorkflowNameError, + NotMaintainedWorkflowError, +) +from exasol.toolbox.util.workflows.templates import ( + NOT_MAINTAINED_WORKFLOW_NAMES, + WORKFLOW_TEMPLATE_OPTIONS, + get_workflow_templates, + validate_workflow_name, +) from noxconfig import PROJECT_CONFIG @@ -32,3 +43,24 @@ def test_get_workflow_templates(project_config): / "workflows" / "build-and-publish.yml" ) + + +class TestValidateWorkflowName: + @staticmethod + @pytest.mark.parametrize( + "workflow_name", + WORKFLOW_TEMPLATE_OPTIONS.keys() - set(NOT_MAINTAINED_WORKFLOW_NAMES), + ) + def test_returns_valid_maintained_names(workflow_name): + name = validate_workflow_name(workflow_name) + assert name == workflow_name + + @staticmethod + def test_rejects_unknown_workflow(): + with pytest.raises(InvalidWorkflowNameError, match="Invalid workflow: unknown"): + validate_workflow_name("unknown") + + @staticmethod + def test_rejects_not_maintained_workflow(): + with pytest.raises(NotMaintainedWorkflowError, match="PTB-seeded workflow"): + validate_workflow_name(NOT_MAINTAINED_WORKFLOW_NAMES[0]) diff --git a/test/unit/util/workflows/workflow_test.py b/test/unit/util/workflows/workflow_test.py index 3c51265df..ea6a308b2 100644 --- a/test/unit/util/workflows/workflow_test.py +++ b/test/unit/util/workflows/workflow_test.py @@ -11,7 +11,10 @@ YamlParsingError, ) from exasol.toolbox.util.workflows.process_template import WorkflowRenderer -from exasol.toolbox.util.workflows.templates import WORKFLOW_TEMPLATE_OPTIONS +from exasol.toolbox.util.workflows.templates import ( + NOT_MAINTAINED_WORKFLOW_NAMES, + WORKFLOW_TEMPLATE_OPTIONS, +) from exasol.toolbox.util.workflows.workflow import ( ALL, Workflow, @@ -183,6 +186,69 @@ def test_works_as_expected_with_not_relevant_patcher( # endpoints, and there are 2 minor whitespace differences. assert result[:10] == input_text[:10] + @staticmethod + def test_not_maintained_workflows_added_to_new_project( + project_config_without_patcher, + ): + directory = project_config_without_patcher.github_workflow_directory + directory.mkdir(parents=True) + + update_workflow(workflow_choice="all", config=project_config_without_patcher) + + assert all( + (directory / f"{name}.yml").exists() + for name in NOT_MAINTAINED_WORKFLOW_NAMES + ) + + @staticmethod + @pytest.mark.parametrize("workflow_name", NOT_MAINTAINED_WORKFLOW_NAMES) + def test_not_maintained_workflows_not_modified_in_old_project( + project_config_without_patcher, workflow_name + ): + directory = project_config_without_patcher.github_workflow_directory + directory.mkdir(parents=True, exist_ok=True) + workflow = "slow-checks.yml" + (directory / workflow).touch() + + update_workflow( + workflow_choice=workflow_name, config=project_config_without_patcher + ) + + assert {file_path.name for file_path in directory.iterdir()} == {workflow} + assert (directory / workflow).read_text() == "" + + @staticmethod + @pytest.mark.parametrize("workflow_name", NOT_MAINTAINED_WORKFLOW_NAMES) + def test_not_maintained_workflows_not_added_to_old_project( + project_config_without_patcher, workflow_name + ): + directory = project_config_without_patcher.github_workflow_directory + directory.mkdir(parents=True, exist_ok=True) + (directory / "dummy.yml").touch() + + update_workflow( + workflow_choice=workflow_name, config=project_config_without_patcher + ) + + assert {file_path.name for file_path in directory.iterdir()} == {"dummy.yml"} + + @staticmethod + @pytest.mark.parametrize("workflow_name", NOT_MAINTAINED_WORKFLOW_NAMES) + def test_not_maintained_workflows_not_modified_in_old_project( + project_config_without_patcher, workflow_name + ): + directory = project_config_without_patcher.github_workflow_directory + directory.mkdir(parents=True, exist_ok=True) + workflow = "slow-checks.yml" + (directory / workflow).touch() + + update_workflow( + workflow_choice=workflow_name, config=project_config_without_patcher + ) + + assert {file_path.name for file_path in directory.iterdir()} == {workflow} + assert (directory / workflow).read_text() == "" + @staticmethod def test_raises_invalidworkflowpatcherentryerror(project_config): patcher_yml = """