Merge pull request #39390 from ClickHouse/paint-it-black

Automatic fixes for black formatting for domestic repo PRs

.github/workflows/docs_check.yml

@@ -102,6 +102,9 @@ jobs:
         run: |
           cat >> "$GITHUB_ENV" << 'EOF'
           TEMP_PATH=${{ runner.temp }}/style_check
+          ROBOT_CLICKHOUSE_SSH_KEY<<RCSK
+          ${{secrets.ROBOT_CLICKHOUSE_SSH_KEY}}
+          RCSK
           EOF
       - name: Download changed images
         # even if artifact does not exist, e.g. on `do not test` label or failed Docker job

.github/workflows/master.yml

@@ -108,7 +108,7 @@ jobs:
       - name: Style Check
         run: |
           cd "$GITHUB_WORKSPACE/tests/ci"
-          python3 style_check.py
+          python3 style_check.py --no-push
       - name: Cleanup
         if: always()
         run: |

.github/workflows/pull_request.yml

@@ -118,6 +118,9 @@ jobs:
         run: |
           cat >> "$GITHUB_ENV" << 'EOF'
           TEMP_PATH=${{ runner.temp }}/style_check
+          ROBOT_CLICKHOUSE_SSH_KEY<<RCSK
+          ${{secrets.ROBOT_CLICKHOUSE_SSH_KEY}}
+          RCSK
           EOF
       - name: Download changed images
         # even if artifact does not exist, e.g. on `do not test` label or failed Docker job

docker/test/style/Dockerfile

@@ -17,7 +17,9 @@ RUN apt-get update && env DEBIAN_FRONTEND=noninteractive apt-get install --yes \
     python3-pip \
     shellcheck \
     yamllint \
-    && pip3 install black boto3 codespell dohq-artifactory PyGithub unidiff pylint==2.6.2
+    && pip3 install black boto3 codespell dohq-artifactory PyGithub unidiff pylint==2.6.2 \
+    && apt-get clean \
+    && rm -rf /root/.cache/pip
 
 # Architecture of the image when BuildKit/buildx is used
 ARG TARGETARCH

docker/test/style/process_style_check_result.py

@@ -40,10 +40,10 @@ def process_result(result_folder):
 
 
 def write_results(results_file, status_file, results, status):
-    with open(results_file, "w") as f:
+    with open(results_file, "w", encoding="utf-8") as f:
         out = csv.writer(f, delimiter="\t")
         out.writerows(results)
-    with open(status_file, "w") as f:
+    with open(status_file, "w", encoding="utf-8") as f:
         out = csv.writer(f, delimiter="\t")
         out.writerow(status)
 
@@ -53,9 +53,10 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser(
         description="ClickHouse script for parsing results of style check"
     )
-    parser.add_argument("--in-results-dir", default="/test_output/")
+    default_dir = "/test_output"
-    parser.add_argument("--out-results-file", default="/test_output/test_results.tsv")
+    parser.add_argument("--in-results-dir", default=default_dir)
-    parser.add_argument("--out-status-file", default="/test_output/check_status.tsv")
+    parser.add_argument("--out-results-file", default=f"{default_dir}/test_results.tsv")
+    parser.add_argument("--out-status-file", default=f"{default_dir}/check_status.tsv")
     args = parser.parse_args()
 
     state, description, test_results = process_result(args.in_results_dir)

tests/ci/style_check.py

@@ -1,32 +1,29 @@
 #!/usr/bin/env python3
-import logging
+import argparse
-import subprocess
-import os
 import csv
+import logging
+import os
+import subprocess
 import sys
 
-from github import Github
 
-from env_helper import (
-    RUNNER_TEMP,
-    GITHUB_WORKSPACE,
-)
-from s3_helper import S3Helper
-from pr_info import PRInfo
-from get_robot_token import get_best_robot_token
-from upload_result_helper import upload_results
-from docker_pull_helper import get_image_with_version
-from commit_status_helper import (
-    post_commit_status,
-    fail_simple_check,
-)
 from clickhouse_helper import (
     ClickHouseHelper,
     mark_flaky_tests,
     prepare_tests_results_for_clickhouse,
 )
-from stopwatch import Stopwatch
+from commit_status_helper import fail_simple_check, post_commit_status
+from docker_pull_helper import get_image_with_version
+from env_helper import GITHUB_WORKSPACE, RUNNER_TEMP
+from get_robot_token import get_best_robot_token
+from github_helper import GitHub
+from git_helper import git_runner
+from pr_info import PRInfo
 from rerun_helper import RerunHelper
+from s3_helper import S3Helper
+from ssh import SSHKey
+from stopwatch import Stopwatch
+from upload_result_helper import upload_results
 
 NAME = "Style Check (actions)"
 
@@ -58,7 +55,8 @@ def process_result(result_folder):
 
     try:
         results_path = os.path.join(result_folder, "test_results.tsv")
-        test_results = list(csv.reader(open(results_path, "r"), delimiter="\t"))
+        with open(results_path, "r", encoding="utf-8") as fd:
+            test_results = list(csv.reader(fd, delimiter="\t"))
         if len(test_results) == 0:
             raise Exception("Empty results")
 
@@ -69,8 +67,77 @@ def process_result(result_folder):
         return state, description, test_results, additional_files
 
 
+def parse_args():
+    parser = argparse.ArgumentParser("Check and report style issues in the repository")
+    parser.add_argument("--push", default=True, help=argparse.SUPPRESS)
+    parser.add_argument(
+        "--no-push",
+        action="store_false",
+        dest="push",
+        help="do not commit and push automatic fixes",
+        default=argparse.SUPPRESS,
+    )
+    return parser.parse_args()
+
+
+def checkout_head(pr_info: PRInfo):
+    # It works ONLY for PRs, and only over ssh, so either
+    # ROBOT_CLICKHOUSE_SSH_KEY should be set or ssh-agent should work
+    assert pr_info.number
+    if not pr_info.head_name == pr_info.base_name:
+        # We can't push to forks, sorry folks
+        return
+    remote_url = pr_info.event["pull_request"]["base"]["repo"]["ssh_url"]
+    git_prefix = (  # All commits to remote are done as robot-clickhouse
+        "git -c user.email=robot-clickhouse@clickhouse.com "
+        "-c user.name=robot-clickhouse -c commit.gpgsign=false "
+        "-c core.sshCommand="
+        "'ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no'"
+    )
+    fetch_cmd = (
+        f"{git_prefix} fetch --depth=1 "
+        f"{remote_url} {pr_info.head_ref}:head-{pr_info.head_ref}"
+    )
+    if os.getenv("ROBOT_CLICKHOUSE_SSH_KEY", ""):
+        with SSHKey("ROBOT_CLICKHOUSE_SSH_KEY"):
+            git_runner(fetch_cmd)
+    else:
+        git_runner(fetch_cmd)
+    git_runner(f"git checkout -f head-{pr_info.head_ref}")
+
+
+def commit_push_staged(pr_info: PRInfo):
+    # It works ONLY for PRs, and only over ssh, so either
+    # ROBOT_CLICKHOUSE_SSH_KEY should be set or ssh-agent should work
+    assert pr_info.number
+    if not pr_info.head_name == pr_info.base_name:
+        # We can't push to forks, sorry folks
+        return
+    git_staged = git_runner("git diff --cached --name-only")
+    if not git_staged:
+        return
+    remote_url = pr_info.event["pull_request"]["base"]["repo"]["ssh_url"]
+    git_prefix = (  # All commits to remote are done as robot-clickhouse
+        "git -c user.email=robot-clickhouse@clickhouse.com "
+        "-c user.name=robot-clickhouse -c commit.gpgsign=false "
+        "-c core.sshCommand="
+        "'ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no'"
+    )
+    git_runner(f"{git_prefix} commit -m 'Automatic style fix'")
+    push_cmd = (
+        f"{git_prefix} push {remote_url} head-{pr_info.head_ref}:{pr_info.head_ref}"
+    )
+    if os.getenv("ROBOT_CLICKHOUSE_SSH_KEY", ""):
+        with SSHKey("ROBOT_CLICKHOUSE_SSH_KEY"):
+            git_runner(push_cmd)
+    else:
+        git_runner(push_cmd)
+
+
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
+    logging.getLogger("git_helper").setLevel(logging.DEBUG)
+    args = parse_args()
 
     stopwatch = Stopwatch()
 
@@ -78,8 +145,10 @@ if __name__ == "__main__":
     temp_path = os.path.join(RUNNER_TEMP, "style_check")
 
     pr_info = PRInfo()
+    if args.push:
+        checkout_head(pr_info)
 
-    gh = Github(get_best_robot_token())
+    gh = GitHub(get_best_robot_token())
 
     rerun_helper = RerunHelper(gh, pr_info, NAME)
     if rerun_helper.is_already_finished_by_status():
@@ -104,6 +173,9 @@ if __name__ == "__main__":
         shell=True,
     )
 
+    if args.push:
+        commit_push_staged(pr_info)
+
     state, description, test_results, additional_files = process_result(temp_path)
     ch_helper = ClickHouseHelper()
     mark_flaky_tests(ch_helper, NAME, test_results)
@@ -111,7 +183,7 @@ if __name__ == "__main__":
     report_url = upload_results(
         s3_helper, pr_info.number, pr_info.sha, test_results, additional_files, NAME
     )
-    print("::notice ::Report url: {}".format(report_url))
+    print(f"::notice ::Report url: {report_url}")
     post_commit_status(gh, pr_info.sha, NAME, description, state, report_url)
 
     prepared_events = prepare_tests_results_for_clickhouse(

utils/check-style/check-black

@@ -6,8 +6,14 @@ set -e
 GIT_ROOT=$(git rev-parse --show-cdup)
 GIT_ROOT=${GIT_ROOT:-.}
 tmp=$(mktemp)
-if ! find "$GIT_ROOT" -name '*.py' -not -path "$GIT_ROOT/contrib/*" -exec black --check --diff {} + 1>"$tmp" 2>&1; then
+# Find all *.py files in the repo except the contrib directory
+find_cmd=(find "$GIT_ROOT" -name '*.py' -not -path "$GIT_ROOT/contrib/*")
+if ! "${find_cmd[@]}" -exec black --check --diff {} + 1>"$tmp" 2>&1; then
   # Show the result only if some files need formatting
   cat "$tmp"
+  # Apply formatting
+  "${find_cmd[@]}" -exec black {} + 1>/dev/null 2>&1
+  # Automatically add changed files to stage
+  "${find_cmd[@]}" -exec git add -u {} + 1>/dev/null 2>&1
 fi
 rm "$tmp"