Merge pull request #55694 from ClickHouse/revert-55476-ci_scripts_tuning

Revert "Integration  check script fix ups"

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

tests/README.md

@@ -1 +0,0 @@
-Find CI documents and instructions on running CI checks localy [here](https://clickhouse.com/docs/en/development/continuous-integration).

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:

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):

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