Skip to content

Commit

Permalink
GH-44878: [Archery] Only suppress Docker progress logs when running o…
Browse files Browse the repository at this point in the history
…n CI (#44865)

### Rationale for this change

#44669 went overboard by always suppressing Docker progress output. This may be desirable on CI, to avoid flooding the logs, but is annoying when running `archery docker` locally.

### What changes are included in this PR?

Only suppress Docker progress output if on CI.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this restores the previous behavior for non-CI runs.

* GitHub Issue: #44878

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
pitrou authored Nov 28, 2024
1 parent 857092c commit 70343d3
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 17 deletions.
11 changes: 8 additions & 3 deletions dev/archery/archery/docker/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from ruamel.yaml import YAML

from ..utils.command import Command, default_bin
from ..utils.logger import running_in_ci
from ..utils.source import arrow_path
from ..compat import _ensure_path

Expand Down Expand Up @@ -168,6 +169,9 @@ def get(self, service_name):
def __getitem__(self, service_name):
return self.get(service_name)

def verbosity_args(self):
return ['--quiet'] if running_in_ci() else []


class Docker(Command):

Expand Down Expand Up @@ -233,7 +237,7 @@ def _execute_docker(self, *args, **kwargs):

def pull(self, service_name, pull_leaf=True, ignore_pull_failures=True):
def _pull(service):
args = ['pull', '--quiet']
args = ['pull'] + self.config.verbosity_args()
if service['image'] in self.pull_memory:
return

Expand Down Expand Up @@ -427,10 +431,11 @@ def run(self, service_name, command=None, *, env=None, volumes=None,

def push(self, service_name, user=None, password=None):
def _push(service):
args = ['push'] + self.config.verbosity_args()
if self.config.using_docker:
return self._execute_docker('push', '--quiet', service['image'])
return self._execute_docker(*args, service['image'])
else:
return self._execute_compose('push', '--quiet', service['name'])
return self._execute_compose(*args, service['name'])

if user is not None:
try:
Expand Down
40 changes: 29 additions & 11 deletions dev/archery/archery/docker/tests/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,14 @@ def arrow_compose_path(tmpdir):
return create_config(tmpdir, arrow_compose_yml, arrow_compose_env)


@pytest.fixture(autouse=True)
def no_ci_env_variables(monkeypatch):
"""Make sure that the tests behave the same on CI as when run locally"""
monkeypatch.delenv("APPVEYOR", raising=False)
monkeypatch.delenv("BUILD_BUILDURI", raising=False)
monkeypatch.delenv("GITHUB_ACTIONS", raising=False)


def test_config_validation(tmpdir):
config_path = create_config(tmpdir, missing_service_compose_yml)
msg = "`sub-foo` is defined in `x-hierarchy` bot not in `services`"
Expand Down Expand Up @@ -270,7 +278,7 @@ def test_compose_default_params_and_env(arrow_compose_path):

def test_forwarding_env_variables(arrow_compose_path):
expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
"pull --ignore-pull-failures conda-cpp",
"build conda-cpp",
]
expected_env = PartialEnv(
Expand All @@ -286,38 +294,48 @@ def test_forwarding_env_variables(arrow_compose_path):
compose.build('conda-cpp')


def test_compose_pull(arrow_compose_path):
def test_compose_pull(arrow_compose_path, monkeypatch):
compose = DockerCompose(arrow_compose_path)

expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
"pull --ignore-pull-failures conda-cpp",
]
with assert_compose_calls(compose, expected_calls):
compose.clear_pull_memory()
compose.pull('conda-cpp')

expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
"pull --quiet --ignore-pull-failures conda-python",
"pull --quiet --ignore-pull-failures conda-python-pandas"
"pull --ignore-pull-failures conda-cpp",
"pull --ignore-pull-failures conda-python",
"pull --ignore-pull-failures conda-python-pandas"
]
with assert_compose_calls(compose, expected_calls):
compose.clear_pull_memory()
compose.pull('conda-python-pandas')

expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
"pull --quiet --ignore-pull-failures conda-python",
"pull --ignore-pull-failures conda-cpp",
"pull --ignore-pull-failures conda-python",
]
with assert_compose_calls(compose, expected_calls):
compose.clear_pull_memory()
compose.pull('conda-python-pandas', pull_leaf=False)

with monkeypatch.context() as m:
# `--quiet` is passed to `docker` on CI
m.setenv("GITHUB_ACTIONS", "true")
expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
]
with assert_compose_calls(compose, expected_calls):
compose.clear_pull_memory()
compose.pull('conda-cpp')


def test_compose_pull_params(arrow_compose_path):
expected_calls = [
"pull --quiet --ignore-pull-failures conda-cpp",
"pull --quiet --ignore-pull-failures conda-python",
"pull --ignore-pull-failures conda-cpp",
"pull --ignore-pull-failures conda-python",
]
compose = DockerCompose(arrow_compose_path, params=dict(UBUNTU='18.04'))
expected_env = PartialEnv(PYTHON='3.8', PANDAS='latest')
Expand Down Expand Up @@ -483,7 +501,7 @@ def test_compose_push(arrow_compose_path):
for image in ["conda-cpp", "conda-python", "conda-python-pandas"]:
expected_calls.append(
mock.call(["docker", "compose", f"--file={compose.config.path}",
"push", "--quiet", image], check=True, env=expected_env)
"push", image], check=True, env=expected_env)
)
with assert_subprocess_calls(expected_calls):
compose.push('conda-python-pandas', user='user', password='pass')
Expand Down
22 changes: 19 additions & 3 deletions dev/archery/archery/utils/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,23 @@ def __init__(self, quiet=False):

ctx = LoggingContext()

in_github_actions = (os.environ.get("GITHUB_ACTIONS") == "true")

# Note: detection routines for many CI services can be found
# in https://github.com/semantic-release/env-ci
def in_appveyor():
return os.environ.get("APPVEYOR", "").lower() == "true"


def in_azure_pipelines():
return os.environ.get("BUILD_BUILDURI", "") != ""


def in_github_actions():
return os.environ.get("GITHUB_ACTIONS") == "true"


def running_in_ci():
return in_appveyor() or in_azure_pipelines() or in_github_actions()


@contextlib.contextmanager
Expand All @@ -43,10 +59,10 @@ def group(name, output=None):
if output is None:
def output(message):
print(message, flush=True)
if in_github_actions:
if in_github_actions():
output(f"::group::{name}")
try:
yield
finally:
if in_github_actions:
if in_github_actions():
output("::endgroup::")

0 comments on commit 70343d3

Please sign in to comment.