From 67b9407530bb15f7e6d49cd1c2bde7b6c441389b Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 16 Oct 2023 22:01:44 +0200 Subject: [PATCH] Revert "Integration check script fix ups" --- docs/en/development/continuous-integration.md | 58 ------------------- tests/README.md | 1 - tests/ci/build_download_helper.py | 42 +++++--------- tests/ci/pr_info.py | 36 +++++++----- tests/ci/worker/prepare-ci-ami.sh | 1 - 5 files changed, 36 insertions(+), 102 deletions(-) delete mode 100644 tests/README.md diff --git a/docs/en/development/continuous-integration.md b/docs/en/development/continuous-integration.md index 46a30f56f11..eec5ccbb9dc 100644 --- a/docs/en/development/continuous-integration.md +++ b/docs/en/development/continuous-integration.md @@ -67,48 +67,6 @@ This check means that the CI system started to process the pull request. When it Performs some simple regex-based checks of code style, using the [`utils/check-style/check-style`](https://github.com/ClickHouse/ClickHouse/blob/master/utils/check-style/check-style) binary (note that it can be run locally). If it fails, fix the style errors following the [code style guide](style.md). -#### Running style check locally: -```sh -mkdir -p /tmp/test_output -# running all checks -docker run --rm --volume=.:/ClickHouse --volume=/tmp/test_output:/test_output -u $(id -u ${USER}):$(id -g ${USER}) --cap-add=SYS_PTRACE clickhouse/style-test - -# run specified check script (e.g.: ./check-mypy) -docker run --rm --volume=.:/ClickHouse --volume=/tmp/test_output:/test_output -u $(id -u ${USER}):$(id -g ${USER}) --cap-add=SYS_PTRACE --entrypoint= -w/ClickHouse/utils/check-style clickhouse/style-test ./check-mypy - -# find all style check scripts under the directory: -cd ./utils/check-style - -# Check duplicate includes -./check-duplicate-includes.sh - -# Check c++ formatiing -./check-style - -# Check python formatting with black -./check-black - -# Check python type hinting with mypy -./check-mypy - -# Check code with codespell -./check-typos - -# Check docs spelling -./check-doc-aspell - -# Check whitespaces -./check-whitespaces - -# Check github actions workflows -./check-workflows - -# Check submodules -./check-submodules - -# Check shell scripts with shellcheck -./shellcheck-run.sh -``` ## Fast Test Normally this is the first check that is ran for a PR. It builds ClickHouse and @@ -117,15 +75,6 @@ some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described [here](tests.md#functional-test-locally). -#### Running Fast Test locally: -```sh -mkdir -p /tmp/test_output -mkdir -p /tmp/fasttest-workspace -cd ClickHouse -# this docker command performs minimal ClickHouse build and run FastTests against it -docker run --rm --cap-add=SYS_PTRACE -u $(id -u ${USER}):$(id -g ${USER}) --network=host -e FASTTEST_WORKSPACE=/fasttest-workspace -e FASTTEST_OUTPUT=/test_output -e FASTTEST_SOURCE=/ClickHouse --cap-add=SYS_PTRACE -e stage=clone_submodules --volume=/tmp/fasttest-workspace:/fasttest-workspace --volume=.:/ClickHouse --volume=/tmp/test_output:/test_output clickhouse/fasttest -``` - #### Status Page Files - `runlog.out.log` is the general log that includes all other logs. @@ -173,13 +122,6 @@ Builds ClickHouse in various configurations for use in further steps. You have t ## Special Build Check Performs static analysis and code style checks using `clang-tidy`. The report is similar to the [build check](#build-check). Fix the errors found in the build log. -#### Running clang-tidy locally: -There is a convenience `packager` script that runs the clang-tidy build in docker -```sh -mkdir build_tidy -./docker/packager/packager --output-dir=./build_tidy --package-type=binary --compiler=clang-17 --debug-build --clang-tidy -``` - ## Functional Stateless Tests Runs [stateless functional tests](tests.md#functional-tests) for ClickHouse diff --git a/tests/README.md b/tests/README.md deleted file mode 100644 index a1fc0f530f2..00000000000 --- a/tests/README.md +++ /dev/null @@ -1 +0,0 @@ -Find CI documents and instructions on running CI checks localy [here](https://clickhouse.com/docs/en/development/continuous-integration). \ No newline at end of file diff --git a/tests/ci/build_download_helper.py b/tests/ci/build_download_helper.py index 21012f6337d..b76c5433142 100644 --- a/tests/ci/build_download_helper.py +++ b/tests/ci/build_download_helper.py @@ -51,9 +51,9 @@ def get_gh_api( sleep: int = 3, **kwargs: Any, ) -> requests.Response: - """ - Request GH api w/o auth by default, and failover to the get_best_robot_token in case of receiving - "403 rate limit exceeded" or "404 not found" error + """It's a wrapper around get_with_retries that requests GH api w/o auth by + default, and falls back to the get_best_robot_token in case of receiving + "403 rate limit exceeded" error It sets auth automatically when ROBOT_TOKEN is already set by get_best_robot_token """ @@ -71,39 +71,27 @@ def get_gh_api( if grt.ROBOT_TOKEN is not None: set_auth_header() - token_is_set = "Authorization" in kwargs.get("headers", {}) - exc = Exception("A placeholder to satisfy typing and avoid nesting") - try_cnt = 0 - while try_cnt < retries: - try_cnt += 1 + need_retry = False + for _ in range(retries): try: - response = requests.get(url, **kwargs) + response = get_with_retries(url, 1, sleep, **kwargs) response.raise_for_status() return response - except requests.HTTPError as e: - exc = e - ratelimit_exceeded = ( - e.response.status_code == 403 + except requests.HTTPError as exc: + if ( + exc.response.status_code == 403 and b"rate limit exceeded" - in e.response._content # pylint:disable=protected-access - ) - try_auth = e.response.status_code == 404 - if (ratelimit_exceeded or try_auth) and not token_is_set: + in exc.response._content # pylint:disable=protected-access + ): logging.warning( "Received rate limit exception, setting the auth header and retry" ) set_auth_header() - token_is_set = True - try_cnt = 0 - continue - except Exception as e: - exc = e + need_retry = True + break - if try_cnt < retries: - logging.info("Exception '%s' while getting, retry %i", exc, try_cnt) - time.sleep(sleep) - - raise exc + if need_retry: + return get_with_retries(url, retries, sleep, **kwargs) def get_build_name_for_check(check_name: str) -> str: diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py index 7dbfe124760..830aa936bea 100644 --- a/tests/ci/pr_info.py +++ b/tests/ci/pr_info.py @@ -4,6 +4,8 @@ import logging import os from typing import Dict, List, Set, Union, Literal +from unidiff import PatchSet # type: ignore + from build_download_helper import get_gh_api from env_helper import ( GITHUB_REPOSITORY, @@ -169,11 +171,7 @@ class PRInfo: response_json = user_orgs_response.json() self.user_orgs = set(org["id"] for org in response_json) - self.diff_urls.append( - f"https://api.github.com/repos/{GITHUB_REPOSITORY}/" - f"compare/master...{self.head_ref}" - ) - + self.diff_urls.append(github_event["pull_request"]["diff_url"]) elif "commits" in github_event: # `head_commit` always comes with `commits` commit_message = github_event["head_commit"]["message"] # type: str @@ -217,12 +215,12 @@ class PRInfo: # files changed in upstream AND master...{self.head_ref} # to get files, changed in current HEAD self.diff_urls.append( - f"https://api.github.com/repos/{GITHUB_REPOSITORY}/" - f"compare/master...{self.head_ref}" + f"https://github.com/{GITHUB_REPOSITORY}/" + f"compare/master...{self.head_ref}.diff" ) self.diff_urls.append( - f"https://api.github.com/repos/{GITHUB_REPOSITORY}/" - f"compare/{self.head_ref}...master" + f"https://github.com/{GITHUB_REPOSITORY}/" + f"compare/{self.head_ref}...master.diff" ) # Get release PR number. self.release_pr = get_pr_for_commit(self.base_ref, self.base_ref)[ @@ -234,8 +232,8 @@ class PRInfo: # For release PRs we must get not only files changed in the PR # itself, but as well files changed since we branched out self.diff_urls.append( - f"https://api.github.com/repos/{GITHUB_REPOSITORY}/" - f"compare/{self.head_ref}...master" + f"https://github.com/{GITHUB_REPOSITORY}/" + f"compare/{self.head_ref}...master.diff" ) else: print("event.json does not match pull_request or push:") @@ -263,11 +261,19 @@ class PRInfo: raise TypeError("The event does not have diff URLs") for diff_url in self.diff_urls: - response = get_gh_api(diff_url, sleep=RETRY_SLEEP) + response = get_gh_api( + diff_url, + sleep=RETRY_SLEEP, + ) response.raise_for_status() - diff = response.json() - if "files" in diff: - self.changed_files = {f["filename"] for f in diff["files"]} + if "commits" in self.event and self.number == 0: + diff = response.json() + + if "files" in diff: + self.changed_files = {f["filename"] for f in diff["files"]} + else: + diff_object = PatchSet(response.text) + self.changed_files.update({f.path for f in diff_object}) print(f"Fetched info about {len(self.changed_files)} changed files") def get_dict(self): diff --git a/tests/ci/worker/prepare-ci-ami.sh b/tests/ci/worker/prepare-ci-ami.sh index f3e11b6d00a..20e7e3fd53e 100644 --- a/tests/ci/worker/prepare-ci-ami.sh +++ b/tests/ci/worker/prepare-ci-ami.sh @@ -90,7 +90,6 @@ systemctl restart docker sudo -u ubuntu docker buildx version sudo -u ubuntu docker buildx create --use --name default-builder -# FIXME: remove unidiff as soon as no old PRs could use it, here and in Dockerfile pip install boto3 pygithub requests urllib3 unidiff dohq-artifactory mkdir -p $RUNNER_HOME && cd $RUNNER_HOME