From 08987700d099338871bd183cdeb8f68903cad719 Mon Sep 17 00:00:00 2001 From: pgoslatara Date: Thu, 15 Aug 2024 10:59:05 +0200 Subject: [PATCH] Moving GitHub PR to gh CLI --- .env.example | 1 - .github/workflows/ci_pipeline.yml | 3 - .gitignore | 1 + action.yml | 10 ++- makefile | 9 --- src/dbt_bouncer/github.py | 101 ------------------------------ src/dbt_bouncer/main.py | 22 ++++--- src/dbt_bouncer/runner.py | 8 +-- src/dbt_bouncer/utils.py | 23 +++++++ tests/github/test_github.py | 34 ---------- tests/unit/test_runner.py | 6 +- tests/unit/test_utils.py | 24 ++++++- 12 files changed, 74 insertions(+), 168 deletions(-) delete mode 100644 src/dbt_bouncer/github.py delete mode 100644 tests/github/test_github.py diff --git a/.env.example b/.env.example index 24fe0641..2181a5f3 100644 --- a/.env.example +++ b/.env.example @@ -1,3 +1,2 @@ export DBT_PROFILES_DIR=dbt_project export DBT_PROJECT_DIR=dbt_project -export GITHUB_TOKEN= # Only required if you are developing the function that posts comments on GitHub PRs, https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#creating-a-fine-grained-personal-access-token diff --git a/.github/workflows/ci_pipeline.yml b/.github/workflows/ci_pipeline.yml index 58c90029..6d71cbe6 100644 --- a/.github/workflows/ci_pipeline.yml +++ b/.github/workflows/ci_pipeline.yml @@ -86,9 +86,6 @@ jobs: - name: Run pytest (integration tests) run: make test-integration - - name: Run pytest (github tests) - run: make test-github - e2e-tests: needs: [pre-commit] runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 8349cb7c..5186ae19 100644 --- a/.gitignore +++ b/.gitignore @@ -163,6 +163,7 @@ cython_debug/ changed_files.txt coverage.json dbt.duckdb +github-comment.md pytest-coverage.txt dbt_project/dbt_packages diff --git a/action.yml b/action.yml index 13aef1bc..07c1b994 100644 --- a/action.yml +++ b/action.yml @@ -41,4 +41,12 @@ runs: ghcr.io/godatadriven/dbt-bouncer:v0.0.0 \ --config-file /app/${{ inputs.config-file }} \ ${{ steps.assemble-output-file-param.outputs.output-file-param }} \ - --send-pr-comment ${{ inputs.send-pr-comment }} + --create-pr-comment-file ${{ inputs.send-pr-comment }} + + - name: Send PR comment + if: ${{ inputs.send-pr-comment }} == 'true' + shell: bash + run: > + gh pr comment ${{ github.pull_request.number }} \ + --repo ${{ github.repository }} \ + --body-file /app/github-comment.md diff --git a/makefile b/makefile index a51d35c4..c768146f 100644 --- a/makefile +++ b/makefile @@ -43,12 +43,3 @@ test-unit: --cov=src/dbt_bouncer/ \ --numprocesses 5 \ ./tests/unit - -test-github: - poetry run pytest \ - -c ./tests \ - --junitxml=coverage.xml \ - --cov-report=term-missing:skip-covered \ - --cov=src/dbt_bouncer/ \ - --numprocesses 5 \ - ./tests/github diff --git a/src/dbt_bouncer/github.py b/src/dbt_bouncer/github.py deleted file mode 100644 index f912f9fe..00000000 --- a/src/dbt_bouncer/github.py +++ /dev/null @@ -1,101 +0,0 @@ -import json -import os -from functools import lru_cache -from typing import Dict, List, Literal, Optional - -import requests -from requests import HTTPError -from requests.auth import AuthBase -from requests.structures import CaseInsensitiveDict - -from dbt_bouncer.logger import logger -from dbt_bouncer.utils import make_markdown_table - - -class GitHubAuth(AuthBase): - """ - Base object to authenticate against the GitHub API. - """ - - def __call__(self, r): - r.headers = CaseInsensitiveDict( - { - "Authorization": f"Bearer {os.environ['GITHUB_TOKEN']}", - "X-GitHub-Api-Version": "2022-11-28", - } - ) - return r - - -def call_github_api( - endpoint: str, - method: Literal["GET", "POST"], - data: Optional[Dict[str, str]] = None, - params: Optional[Dict[str, str]] = None, -) -> Dict[str, str]: - """Call the GitHub API with a GET request and return the response as a dict. - - Args: - endpoint (str): Can contain "https://api.github.com/" in the beginning, or not. - method (Literal["GET", "POST"]): GET or POST. - data (Dict, optional): Defaults to None. - params (Dict, optional): Defaults to None. - - Returns: - Dict - """ - - if method == "GET": - r = create_requests_session().get( - auth=GitHubAuth(), - params=params, - url=f"https://api.github.com/{endpoint}", - ) - elif method == "POST": - r = create_requests_session().post( - auth=GitHubAuth(), - data=json.dumps(data), - url=f"https://api.github.com/{endpoint}", - ) - - try: - r.raise_for_status() - except HTTPError as e: - logger.error(f"{r.status_code=}") - logger.error(f"{r.content=}") - raise RuntimeError(e) from e - - logger.debug(f"Response: {r.status_code}, {r.reason}") - - return r.json() - - -@lru_cache -def create_requests_session() -> requests.Session: - """ - Create a requests session and cache it to avoid recreating the session. - """ - - logger.debug("Creating reusable requests session...") - return requests.Session() - - -def send_github_comment_failed_checks(failed_checks: List[List[str]]) -> None: - """ - Send a GitHub comment to the PR with a list of failed checks. - """ - - md_formatted_comment = make_markdown_table( - [["Check name", "Failure message"]] + list(sorted(failed_checks)) - ) - - md_formatted_comment = f"## **Failed `dbt-bouncer`** checks\n\n{md_formatted_comment}\n\nSent from this [GitHub Action workflow run](https://github.com/{os.environ['GITHUB_REPOSITORY']}/actions/runs/{os.environ['GITHUB_RUN_ID']})." # Would like to be more specific and include the job ID, but it's not exposed as an environment variable: https://github.com/actions/runner/issues/324 - - logger.debug(f"{md_formatted_comment=}") - - r = call_github_api( - data={"body": md_formatted_comment}, - endpoint=f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['GITHUB_REF'].split('/')[-2]}/comments", - method="POST", - ) - logger.info(f"Comment URL: {r['html_url']}") diff --git a/src/dbt_bouncer/main.py b/src/dbt_bouncer/main.py index bdec4953..a7f083df 100644 --- a/src/dbt_bouncer/main.py +++ b/src/dbt_bouncer/main.py @@ -21,6 +21,13 @@ required=False, type=Path, ) +@click.option( + "--create-pr-comment-file", + default=False, + help="Create a `github-comment.md` file that will be sent to GitHub as a PR comment. Defaults to True when run as a GitHub Action.", + required=False, + type=click.BOOL, +) @click.option( "--output-file", default=None, @@ -28,15 +35,12 @@ required=False, type=Path, ) -@click.option( - "--send-pr-comment", - default=False, - help="Send a comment to the GitHub PR with a list of failed checks. Defaults to True when run as a GitHub Action.", - required=False, - type=click.BOOL, -) @click.version_option() -def cli(config_file: Path, output_file: Union[None, Path], send_pr_comment: bool): +def cli( + config_file: Path, + create_pr_comment_file: bool, + output_file: Union[None, Path], +): logger.info(f"Running dbt-bouncer ({version()})...") # Validate output file has `.json` extension @@ -168,13 +172,13 @@ def cli(config_file: Path, output_file: Union[None, Path], send_pr_comment: bool bouncer_config=config, catalog_nodes=project_catalog_nodes, catalog_sources=project_catalog_sources, + create_pr_comment_file=create_pr_comment_file, exposures=project_exposures, macros=project_macros, manifest_obj=manifest_obj, models=project_models, output_file=output_file, run_results=project_run_results, - send_pr_comment=send_pr_comment, sources=project_sources, tests=project_tests, ) diff --git a/src/dbt_bouncer/runner.py b/src/dbt_bouncer/runner.py index b7637247..c54ca829 100644 --- a/src/dbt_bouncer/runner.py +++ b/src/dbt_bouncer/runner.py @@ -8,26 +8,26 @@ import pytest from tabulate import tabulate -from dbt_bouncer.github import send_github_comment_failed_checks from dbt_bouncer.logger import logger from dbt_bouncer.runner_plugins import ( FixturePlugin, GenerateTestsPlugin, ResultsCollector, ) +from dbt_bouncer.utils import create_github_comment_file def runner( bouncer_config: Dict[str, List[Dict[str, str]]], catalog_nodes: List[Dict[str, str]], catalog_sources: List[Dict[str, str]], + create_pr_comment_file: bool, exposures: List[Dict[str, str]], macros: List[Dict[str, str]], manifest_obj: Dict[str, str], models: List[Dict[str, str]], output_file: Union[None, Path], run_results: List[Dict[str, str]], - send_pr_comment: bool, sources: List[Dict[str, str]], tests: List[Dict[str, str]], checks_dir: Optional[Union[None, Path]] = Path(__file__).parent / "checks", @@ -95,8 +95,8 @@ def runner( ) ) - if send_pr_comment: - send_github_comment_failed_checks(failed_checks=failed_checks) + if create_pr_comment_file: + create_github_comment_file(failed_checks=failed_checks) if output_file is not None: coverage_file = Path().cwd() / output_file diff --git a/src/dbt_bouncer/utils.py b/src/dbt_bouncer/utils.py index fa79d062..677a30ee 100644 --- a/src/dbt_bouncer/utils.py +++ b/src/dbt_bouncer/utils.py @@ -1,5 +1,6 @@ import contextlib import json +import os import re from pathlib import Path from typing import List, Literal @@ -11,6 +12,28 @@ from dbt_bouncer.logger import logger +def create_github_comment_file(failed_checks: List[List[str]]) -> None: + """ + Create a markdown file containing a comment for GitHub. + """ + + md_formatted_comment = make_markdown_table( + [["Check name", "Failure message"]] + list(sorted(failed_checks)) + ) + + md_formatted_comment = f"## **Failed `dbt-bouncer`** checks\n\n{md_formatted_comment}\n\nSent from this [GitHub Action workflow run](https://github.com/{os.environ['GITHUB_REPOSITORY']}/actions/runs/{os.environ['GITHUB_RUN_ID']})." # Would like to be more specific and include the job ID, but it's not exposed as an environment variable: https://github.com/actions/runner/issues/324 + + logger.debug(f"{md_formatted_comment=}") + + if os.environ.get("CI", None): + comment_file = "/app/github-comment.md" + else: + comment_file = "github-comment.md" + + with open(comment_file, "w") as f: + f.write(md_formatted_comment) + + def find_missing_meta_keys(meta_config, required_keys) -> List[str]: """ Find missing keys in a meta config. diff --git a/tests/github/test_github.py b/tests/github/test_github.py deleted file mode 100644 index ccccbd6d..00000000 --- a/tests/github/test_github.py +++ /dev/null @@ -1,34 +0,0 @@ -import pytest -import requests - -from dbt_bouncer.github import call_github_api, create_requests_session -from dbt_bouncer.logger import logger - - -def test_create_requests_session(): - """ - Test that the GitHub API can be reached - """ - assert isinstance(create_requests_session(), requests.Session) - - -def test_github_api_connection(): - """ - Test that the GitHub API can be reached - """ - - response = call_github_api( - endpoint="repos/godatadriven/dbt-bouncer", # Unfortunately this endpoint doesn't return a `success` key, - method="GET", - ) - logger.warning(response) - assert isinstance(response["id"], int) - - -def test_github_api_connection_nonexisting_endpoint(): - """ - Test that the GitHub API can be reached - """ - - with pytest.raises(Exception): - call_github_api(endpoint="I_don_t_exist", method="GET") diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index cb092f09..dee880ff 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -15,6 +15,7 @@ def test_runner_coverage(caplog, tmp_path): }, catalog_nodes=[], catalog_sources=[], + create_pr_comment_file=False, exposures=[], macros=[], manifest_obj=parse_manifest( @@ -49,7 +50,6 @@ def test_runner_coverage(caplog, tmp_path): ], output_file=tmp_path / "coverage.json", run_results=[], - send_pr_comment=False, sources=[], tests=[], checks_dir=Path("./src/dbt_bouncer/checks"), @@ -73,6 +73,7 @@ def test_runner_failure(): }, catalog_nodes=[], catalog_sources=[], + create_pr_comment_file=False, exposures=[], macros=[], manifest_obj=parse_manifest( @@ -107,7 +108,6 @@ def test_runner_failure(): ], output_file=None, run_results=[], - send_pr_comment=False, sources=[], tests=[], checks_dir=Path("./src/dbt_bouncer/checks"), @@ -124,6 +124,7 @@ def test_runner_success(): }, catalog_nodes=[], catalog_sources=[], + create_pr_comment_file=False, exposures=[], macros=[], manifest_obj=parse_manifest( @@ -158,7 +159,6 @@ def test_runner_success(): ], output_file=None, run_results=[], - send_pr_comment=False, sources=[], tests=[], checks_dir=Path("./src/dbt_bouncer/checks"), diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 4040e0bf..988b5c26 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -2,8 +2,10 @@ import pytest import toml +from pytest import MonkeyPatch from dbt_bouncer.utils import ( + create_github_comment_file, flatten, get_dbt_bouncer_config, make_markdown_table, @@ -11,6 +13,24 @@ ) +def test_create_github_comment_file(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + + with MonkeyPatch.context() as mp: + mp.setenv("GITHUB_REPOSITORY", "org/repo") + mp.setenv("GITHUB_RUN_ID", "123") + + failed_checks = [ + ["check_model_description_populated", "message_1"], + ["check_model_description_populated", "message_2"], + ] + create_github_comment_file(failed_checks) + assert ( + Path("github-comment.md").read_text() + == "## **Failed `dbt-bouncer`** checks\n\n\n| Check name | Failure message |\n| :--- | :--- |\n| check_model_description_populated | message_1 |\n| check_model_description_populated | message_2 |\n\n\nSent from this [GitHub Action workflow run](https://github.com/org/repo/actions/runs/123)." + ) + + def test_get_dbt_bouncer_config_commandline(tmp_path): config_file = tmp_path / "my_dbt_bouncer.yml" config_file.write_text("test: 1") @@ -57,9 +77,7 @@ def test_get_dbt_bouncer_config_pyproject_toml_doesnt_exist(monkeypatch, tmp_pat monkeypatch.chdir(tmp_path) with pytest.raises(RuntimeError): - config = get_dbt_bouncer_config( - config_file=str("dbt_bouncer.yml"), config_file_source="DEFAULT" - ) + get_dbt_bouncer_config(config_file=str("dbt_bouncer.yml"), config_file_source="DEFAULT") def test_get_dbt_bouncer_config_pyproject_toml_recursive(monkeypatch, tmp_path):