From 619c7c3875783c8982f40b2db3bf921ef7cfae6e Mon Sep 17 00:00:00 2001 From: Jonny Browning Date: Tue, 25 Oct 2022 14:18:44 +0100 Subject: [PATCH 1/2] feat: added --build-image/--no-build-image flags to skip docker build --- sdk/python/kfp/cli/component.py | 19 ++++++++++++++----- sdk/python/kfp/cli/component_test.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/sdk/python/kfp/cli/component.py b/sdk/python/kfp/cli/component.py index 2936fb06c0f..0156fa085e1 100644 --- a/sdk/python/kfp/cli/component.py +++ b/sdk/python/kfp/cli/component.py @@ -361,6 +361,12 @@ def component(): default=False, help='Set this to true to always generate a Dockerfile' ' as part of the build process') +@click.option( + '--build-image/--no-build-image', + type=bool, + is_flag=True, + default=True, + help='Build the container image.') @click.option( '--push-image/--no-push-image', type=bool, @@ -369,10 +375,10 @@ def component(): help='Push the built image to its remote repository.') def build(components_directory: str, component_filepattern: str, engine: str, kfp_package_path: Optional[str], overwrite_dockerfile: bool, - push_image: bool): + build_image: bool, push_image: bool): """Builds containers for KFP v2 Python-based components.""" - if engine != 'docker': + if not build_image and engine != 'docker': warnings.warn( 'The --engine option is deprecated and does not need to be passed. Only Docker engine is supported and will be used by default.', DeprecationWarning, @@ -387,12 +393,14 @@ def build(components_directory: str, component_filepattern: str, engine: str, f'{components_directory} does not seem to be a valid directory.') raise sys.exit(1) - if not _DOCKER_IS_PRESENT: + if not build_image and not _DOCKER_IS_PRESENT: logging.error( 'The `docker` Python package was not found in the current' ' environment. Please run `pip install docker` to install it.' ' Optionally, you can also install KFP with all of its' - ' optional dependencies by running `pip install kfp[all]`.') + ' optional dependencies by running `pip install kfp[all]`.' + ' Alternatively, you can skip the container image build using' + ' the --no-build-image flag.') raise sys.exit(1) kfp_package_path = pathlib.Path( @@ -408,4 +416,5 @@ def build(components_directory: str, component_filepattern: str, engine: str, builder.generate_requirements_txt() builder.maybe_generate_dockerignore() builder.maybe_generate_dockerfile(overwrite_dockerfile=overwrite_dockerfile) - builder.build_image(push_image=push_image) + if build_image: + builder.build_image(push_image=push_image) diff --git a/sdk/python/kfp/cli/component_test.py b/sdk/python/kfp/cli/component_test.py index 1c763fd8e0a..6c17e39fd0c 100644 --- a/sdk/python/kfp/cli/component_test.py +++ b/sdk/python/kfp/cli/component_test.py @@ -386,6 +386,20 @@ def test_docker_client_is_called_to_build_and_push_by_default(self): self._docker_client.images.push.assert_called_once_with( 'custom-image', stream=True, decode=True) + def test_docker_client_is_not_called_to_build_or_push(self): + component = _make_component( + func_name='train', target_image='custom-image') + _write_components('components.py', component) + + result = self.runner.invoke( + self.cli, + ['build', str(self._working_dir), '--no-build-image'], + ) + self.assertEqual(result.exit_code, 0) + + self._docker_client.api.build.assert_not_called() + self._docker_client.images.push.assert_not_called() + def test_docker_client_is_called_to_build_but_skips_pushing(self): component = _make_component( func_name='train', target_image='custom-image') From eb1dc7fac613228535efb2f820e7c37dfc5e8804 Mon Sep 17 00:00:00 2001 From: Jonny Browning Date: Wed, 26 Oct 2022 09:24:48 +0100 Subject: [PATCH 2/2] fix: logic error for warnings/errors --- sdk/python/kfp/cli/component.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/kfp/cli/component.py b/sdk/python/kfp/cli/component.py index 0156fa085e1..0546e609285 100644 --- a/sdk/python/kfp/cli/component.py +++ b/sdk/python/kfp/cli/component.py @@ -378,7 +378,7 @@ def build(components_directory: str, component_filepattern: str, engine: str, build_image: bool, push_image: bool): """Builds containers for KFP v2 Python-based components.""" - if not build_image and engine != 'docker': + if build_image and engine != 'docker': warnings.warn( 'The --engine option is deprecated and does not need to be passed. Only Docker engine is supported and will be used by default.', DeprecationWarning, @@ -393,7 +393,7 @@ def build(components_directory: str, component_filepattern: str, engine: str, f'{components_directory} does not seem to be a valid directory.') raise sys.exit(1) - if not build_image and not _DOCKER_IS_PRESENT: + if build_image and not _DOCKER_IS_PRESENT: logging.error( 'The `docker` Python package was not found in the current' ' environment. Please run `pip install docker` to install it.'