From e543e222266fd28ea648f7740d93ea026fdf5c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 12:44:24 +0200 Subject: [PATCH 1/7] ARROW-17953: [Archery] Add archery docker info command --- dev/archery/archery/docker/cli.py | 17 +++++++++++++++++ dev/archery/archery/docker/core.py | 10 +++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index bbdd2261db68..c98de2cb48e2 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -289,3 +289,20 @@ def docker_compose_images(obj): click.echo('Available images:') for image in compose.images(): click.echo(f' - {image}') + +@docker.command('info') +@click.option('--service', '-s', 'service_name', required=True, + help='Service name to show info') +@click.pass_obj +def docker_compose_info(obj, service_name): + """List the available docker-compose images.""" + compose = obj['compose'] + click.echo(f'Service {service_name} docker compose config:') + def expand(k, prefix=' '): + if hasattr(k, 'items'): + for key, value in k.items(): + click.echo(f'{prefix}- {key}') + expand(value, prefix + " ") + else: + click.echo(f'{prefix}- {k}') + expand(compose.config.raw_config["services"][service_name]) \ No newline at end of file diff --git a/dev/archery/archery/docker/core.py b/dev/archery/archery/docker/core.py index da15f86935b4..1e832ab59562 100644 --- a/dev/archery/archery/docker/core.py +++ b/dev/archery/archery/docker/core.py @@ -95,12 +95,12 @@ def _read_config(self, config_path, compose_bin): """ yaml = YAML() with config_path.open() as fp: - config = yaml.load(fp) + self.raw_config = yaml.load(fp) - services = config['services'].keys() - self.hierarchy = dict(flatten(config.get('x-hierarchy', {}))) - self.limit_presets = config.get('x-limit-presets', {}) - self.with_gpus = config.get('x-with-gpus', []) + services = self.raw_config['services'].keys() + self.hierarchy = dict(flatten(self.raw_config.get('x-hierarchy', {}))) + self.limit_presets = self.raw_config.get('x-limit-presets', {}) + self.with_gpus = self.raw_config.get('x-with-gpus', []) nodes = self.hierarchy.keys() errors = [] From 509dc5de1bc5484dfed09479434c3899d8731930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 17:08:54 +0200 Subject: [PATCH 2/7] Add possibility of filtering parts of the docker-compose info --- dev/archery/archery/docker/cli.py | 45 ++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index c98de2cb48e2..ef0cd9daf53d 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -290,19 +290,38 @@ def docker_compose_images(obj): for image in compose.images(): click.echo(f' - {image}') + @docker.command('info') -@click.option('--service', '-s', 'service_name', required=True, - help='Service name to show info') +@click.argument('service_name') +@click.option('--only', '-o', required=False, + help="Show only specific docker-compose key. Examples of keys:" + " command, environment, build, dockerfile") @click.pass_obj -def docker_compose_info(obj, service_name): - """List the available docker-compose images.""" +def docker_compose_info(obj, service_name, only): + """Show docker-compose definition info for service_name. + + SERVICE_NAME is the name of the docker service defined on + the docker-compose. Look at `archery docker images` output for names. + """ compose = obj['compose'] - click.echo(f'Service {service_name} docker compose config:') - def expand(k, prefix=' '): - if hasattr(k, 'items'): - for key, value in k.items(): - click.echo(f'{prefix}- {key}') - expand(value, prefix + " ") - else: - click.echo(f'{prefix}- {k}') - expand(compose.config.raw_config["services"][service_name]) \ No newline at end of file + + def expand(k, filters=None, prefix=' '): + for key, value in k.items(): + if hasattr(value, 'items'): + keep_filters = filters + if key == filters or filters is None: + click.echo(f'{prefix}- {key}') + # Keep showing this specific key as parent matched filter + keep_filters = None + expand(value, keep_filters, prefix + " ") + else: + if key == filters or filters is None: + click.echo(f'{prefix}- {key}: {value}') + + try: + service = compose.config.raw_config["services"][service_name] + except KeyError: + click.echo(f'Service name {service_name} could not be found') + else: + click.echo(f'Service {service_name} docker compose config:') + expand(service, only) From e5035616ac1befba1f8efcb53b7859c44a4586f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 17:58:02 +0200 Subject: [PATCH 3/7] Move functionality to DockerCompose and add tests --- dev/archery/archery/docker/cli.py | 17 ++----------- dev/archery/archery/docker/core.py | 16 ++++++++++++ .../archery/docker/tests/test_docker.py | 25 +++++++++++++++++++ 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index ef0cd9daf53d..ea14a99a047f 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -304,24 +304,11 @@ def docker_compose_info(obj, service_name, only): the docker-compose. Look at `archery docker images` output for names. """ compose = obj['compose'] - - def expand(k, filters=None, prefix=' '): - for key, value in k.items(): - if hasattr(value, 'items'): - keep_filters = filters - if key == filters or filters is None: - click.echo(f'{prefix}- {key}') - # Keep showing this specific key as parent matched filter - keep_filters = None - expand(value, keep_filters, prefix + " ") - else: - if key == filters or filters is None: - click.echo(f'{prefix}- {key}: {value}') - try: service = compose.config.raw_config["services"][service_name] except KeyError: click.echo(f'Service name {service_name} could not be found') else: click.echo(f'Service {service_name} docker compose config:') - expand(service, only) + output = "\n".join(compose.info(service, only)) + click.echo(output) diff --git a/dev/archery/archery/docker/core.py b/dev/archery/archery/docker/core.py index 1e832ab59562..a3056d71b447 100644 --- a/dev/archery/archery/docker/core.py +++ b/dev/archery/archery/docker/core.py @@ -417,3 +417,19 @@ def _push(service): def images(self): return sorted(self.config.hierarchy.keys()) + + def info(self, key_name, filters=None, prefix=' '): + output = [] + for key, value in key_name.items(): + if hasattr(value, 'items'): + temp_filters = filters + if key == filters or filters is None: + output.append(f'{prefix}- {key}') + # Keep showing this specific key + # as parent matched filter + temp_filters = None + output.extend(self.info(value, temp_filters, prefix + " ")) + else: + if key == filters or filters is None: + output.append(f'{prefix}- {key}: {value}') + return output diff --git a/dev/archery/archery/docker/tests/test_docker.py b/dev/archery/archery/docker/tests/test_docker.py index 899a0449e1a7..23c3b3c17d8d 100644 --- a/dev/archery/archery/docker/tests/test_docker.py +++ b/dev/archery/archery/docker/tests/test_docker.py @@ -529,3 +529,28 @@ def test_listing_images(arrow_compose_path): 'ubuntu-cuda', 'ubuntu-ruby', ] + + +def test_service_info(arrow_compose_path): + compose = DockerCompose(arrow_compose_path) + service = compose.config.raw_config["services"]["conda-cpp"] + assert compose.info(service) == [ + " - image: org/conda-cpp", + " - build", + " - context: .", + " - dockerfile: ci/docker/conda-cpp.dockerfile" + ] + + +def test_service_info_filters(arrow_compose_path): + compose = DockerCompose(arrow_compose_path) + service = compose.config.raw_config["services"]["conda-cpp"] + assert compose.info(service, filters="dockerfile") == [ + " - dockerfile: ci/docker/conda-cpp.dockerfile" + ] + + +def test_service_info_non_existing_filters(arrow_compose_path): + compose = DockerCompose(arrow_compose_path) + service = compose.config.raw_config["services"]["conda-cpp"] + assert compose.info(service, filters="non-existing") == [] From b8b4485b45057e9a3c8e4b00f777e48f5ab95443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Oct 2022 13:04:54 +0200 Subject: [PATCH 4/7] Update dev/archery/archery/docker/cli.py Co-authored-by: Antoine Pitrou --- dev/archery/archery/docker/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index ea14a99a047f..27f2aa8ffa96 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -307,7 +307,8 @@ def docker_compose_info(obj, service_name, only): try: service = compose.config.raw_config["services"][service_name] except KeyError: - click.echo(f'Service name {service_name} could not be found') + click.echo(f'Service name {service_name} could not be found', err=True) + sys.exit(1) else: click.echo(f'Service {service_name} docker compose config:') output = "\n".join(compose.info(service, only)) From 8fb2c3320fe9dae4bbcf6c9e442bc9ad8bb0fe03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Oct 2022 13:05:01 +0200 Subject: [PATCH 5/7] Update dev/archery/archery/docker/cli.py Co-authored-by: Antoine Pitrou --- dev/archery/archery/docker/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index 27f2aa8ffa96..6922214a4dff 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -310,6 +310,6 @@ def docker_compose_info(obj, service_name, only): click.echo(f'Service name {service_name} could not be found', err=True) sys.exit(1) else: - click.echo(f'Service {service_name} docker compose config:') + click.echo(f'Service {service_name} docker-compose config:') output = "\n".join(compose.info(service, only)) click.echo(output) From 9dde65df9d0d650c9a6f62f9365b4ef47a460eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Oct 2022 13:16:24 +0200 Subject: [PATCH 6/7] Apply code review suggestions to improve UX for command and output --- dev/archery/archery/docker/cli.py | 7 ++++--- dev/archery/archery/docker/core.py | 6 +++--- dev/archery/archery/docker/tests/test_docker.py | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/dev/archery/archery/docker/cli.py b/dev/archery/archery/docker/cli.py index 6922214a4dff..c7b42c094f61 100644 --- a/dev/archery/archery/docker/cli.py +++ b/dev/archery/archery/docker/cli.py @@ -16,6 +16,7 @@ # under the License. import os +import sys import click @@ -293,11 +294,11 @@ def docker_compose_images(obj): @docker.command('info') @click.argument('service_name') -@click.option('--only', '-o', required=False, +@click.option('--show', '-s', required=False, help="Show only specific docker-compose key. Examples of keys:" " command, environment, build, dockerfile") @click.pass_obj -def docker_compose_info(obj, service_name, only): +def docker_compose_info(obj, service_name, show): """Show docker-compose definition info for service_name. SERVICE_NAME is the name of the docker service defined on @@ -311,5 +312,5 @@ def docker_compose_info(obj, service_name, only): sys.exit(1) else: click.echo(f'Service {service_name} docker-compose config:') - output = "\n".join(compose.info(service, only)) + output = "\n".join(compose.info(service, show)) click.echo(output) diff --git a/dev/archery/archery/docker/core.py b/dev/archery/archery/docker/core.py index a3056d71b447..ebd30cd41323 100644 --- a/dev/archery/archery/docker/core.py +++ b/dev/archery/archery/docker/core.py @@ -424,12 +424,12 @@ def info(self, key_name, filters=None, prefix=' '): if hasattr(value, 'items'): temp_filters = filters if key == filters or filters is None: - output.append(f'{prefix}- {key}') + output.append(f'{prefix} {key}') # Keep showing this specific key # as parent matched filter temp_filters = None - output.extend(self.info(value, temp_filters, prefix + " ")) + output.extend(self.info(value, temp_filters, prefix + " ")) else: if key == filters or filters is None: - output.append(f'{prefix}- {key}: {value}') + output.append(f'{prefix} {key}: {value}') return output diff --git a/dev/archery/archery/docker/tests/test_docker.py b/dev/archery/archery/docker/tests/test_docker.py index 23c3b3c17d8d..d20e89b3c250 100644 --- a/dev/archery/archery/docker/tests/test_docker.py +++ b/dev/archery/archery/docker/tests/test_docker.py @@ -535,10 +535,10 @@ def test_service_info(arrow_compose_path): compose = DockerCompose(arrow_compose_path) service = compose.config.raw_config["services"]["conda-cpp"] assert compose.info(service) == [ - " - image: org/conda-cpp", - " - build", - " - context: .", - " - dockerfile: ci/docker/conda-cpp.dockerfile" + " image: org/conda-cpp", + " build", + " context: .", + " dockerfile: ci/docker/conda-cpp.dockerfile" ] @@ -546,7 +546,7 @@ def test_service_info_filters(arrow_compose_path): compose = DockerCompose(arrow_compose_path) service = compose.config.raw_config["services"]["conda-cpp"] assert compose.info(service, filters="dockerfile") == [ - " - dockerfile: ci/docker/conda-cpp.dockerfile" + " dockerfile: ci/docker/conda-cpp.dockerfile" ] From 28e9408d5b63f022c4705847b91697c8fccc700c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Tue, 18 Oct 2022 15:05:05 +0200 Subject: [PATCH 7/7] ARROW-17953: [Archery] Use keyword if the value for docker info command is None as it would be inherited from shell environment values --- dev/archery/archery/docker/core.py | 5 ++++- .../archery/docker/tests/test_docker.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/dev/archery/archery/docker/core.py b/dev/archery/archery/docker/core.py index ebd30cd41323..de5e6cf41c9a 100644 --- a/dev/archery/archery/docker/core.py +++ b/dev/archery/archery/docker/core.py @@ -431,5 +431,8 @@ def info(self, key_name, filters=None, prefix=' '): output.extend(self.info(value, temp_filters, prefix + " ")) else: if key == filters or filters is None: - output.append(f'{prefix} {key}: {value}') + output.append( + f'{prefix} {key}: ' + + f'{value if value is not None else ""}' + ) return output diff --git a/dev/archery/archery/docker/tests/test_docker.py b/dev/archery/archery/docker/tests/test_docker.py index d20e89b3c250..bc25738becf7 100644 --- a/dev/archery/archery/docker/tests/test_docker.py +++ b/dev/archery/archery/docker/tests/test_docker.py @@ -114,6 +114,11 @@ arrow_compose_yml = """ version: '3.5' +x-sccache: &sccache + AWS_ACCESS_KEY_ID: + AWS_SECRET_ACCESS_KEY: + SCCACHE_BUCKET: + x-with-gpus: - ubuntu-cuda @@ -162,6 +167,8 @@ image: org/ubuntu-cpp-cmake32 ubuntu-c-glib: image: org/ubuntu-c-glib + environment: + <<: [*sccache] ubuntu-ruby: image: org/ubuntu-ruby ubuntu-cuda: @@ -554,3 +561,14 @@ def test_service_info_non_existing_filters(arrow_compose_path): compose = DockerCompose(arrow_compose_path) service = compose.config.raw_config["services"]["conda-cpp"] assert compose.info(service, filters="non-existing") == [] + + +def test_service_info_inherited_env(arrow_compose_path): + compose = DockerCompose(arrow_compose_path) + service = compose.config.raw_config["services"]["ubuntu-c-glib"] + assert compose.info(service, filters="environment") == [ + " environment", + " AWS_ACCESS_KEY_ID: ", + " AWS_SECRET_ACCESS_KEY: ", + " SCCACHE_BUCKET: " + ]