From 917060ecbc169fa2d7f6f01dd9656334cab734e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emirhan=20Karag=C3=BCl?= Date: Tue, 2 Nov 2021 10:52:57 +0100 Subject: [PATCH 1/6] Add default entrypointsettings to remote serialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Emirhan Karagül --- flytekit/remote/remote.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/flytekit/remote/remote.py b/flytekit/remote/remote.py index d031bc6aa0..16c650e43d 100644 --- a/flytekit/remote/remote.py +++ b/flytekit/remote/remote.py @@ -37,13 +37,20 @@ from flytekit.clients.helpers import iterate_node_executions, iterate_task_executions from flytekit.clis.flyte_cli.main import _detect_default_config_file +from flytekit.clis.sdk_in_container import serialize from flytekit.common import constants from flytekit.common.exceptions import user as user_exceptions from flytekit.common.translator import FlyteControlPlaneEntity, FlyteLocalEntity, get_serializable from flytekit.configuration import auth as auth_config from flytekit.configuration.internal import DOMAIN, PROJECT from flytekit.core.base_task import PythonTask -from flytekit.core.context_manager import FlyteContextManager, ImageConfig, SerializationSettings, get_image_config +from flytekit.core.context_manager import ( + EntrypointSettings, + FlyteContextManager, + ImageConfig, + SerializationSettings, + get_image_config, +) from flytekit.core.data_persistence import FileAccessProvider from flytekit.core.launch_plan import LaunchPlan from flytekit.core.type_engine import TypeEngine @@ -527,6 +534,11 @@ def _serialize( self.image_config, # https://github.com/flyteorg/flyte/issues/1359 env={internal.IMAGE.env_var: self.image_config.default_image.full}, + entrypoint_settings=EntrypointSettings( + path=os.path.join( + serialize._DEFAULT_FLYTEKIT_VIRTUALENV_ROOT, serialize._DEFAULT_FLYTEKIT_RELATIVE_ENTRYPOINT_LOC + ) + ), ), entity=entity, ) From eafaaab8768f859aeb355430be8b96249405989b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emirhan=20Karag=C3=BCl?= Date: Tue, 2 Nov 2021 12:22:29 +0100 Subject: [PATCH 2/6] Add unit test to check spark job template with remote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Emirhan Karagül --- .../tests/test_remote_register.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 plugins/flytekit-spark/tests/test_remote_register.py diff --git a/plugins/flytekit-spark/tests/test_remote_register.py b/plugins/flytekit-spark/tests/test_remote_register.py new file mode 100644 index 0000000000..cb037985cf --- /dev/null +++ b/plugins/flytekit-spark/tests/test_remote_register.py @@ -0,0 +1,30 @@ +from flytekitplugins.spark import Spark +from mock import MagicMock, patch + +from flytekit import task +from flytekit.remote.remote import FlyteRemote + + +@patch("flytekit.configuration.platform.URL") +@patch("flytekit.configuration.platform.INSECURE") +def test_spark_template_with_remote(mock_insecure, mock_url): + @task(task_config=Spark(spark_conf={"spark": "1"})) + def my_spark(a: str) -> int: + return 10 + + mock_url.get.return_value = "localhost" + + mock_insecure.get.return_value = True + mock_client = MagicMock() + + remote = FlyteRemote.from_config("p1", "d1") + + remote._image_config = MagicMock() + remote._client = mock_client + + remote.register(my_spark) + serialized_spec = mock_client.create_task.call_args.kwargs["task_spec"] + + # Check if the serialized task has mainApplicaitonFile field set. + assert serialized_spec.template.custom["mainApplicationFile"] + assert serialized_spec.template.custom["sparkConf"] From 39597eda44a4bc620be748dab16002eb12735790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emirhan=20Karag=C3=BCl?= Date: Wed, 3 Nov 2021 08:47:40 +0100 Subject: [PATCH 3/6] Extend unittest to ensure no extra field is being set for python task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Emirhan Karagül --- plugins/flytekit-spark/tests/test_remote_register.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/flytekit-spark/tests/test_remote_register.py b/plugins/flytekit-spark/tests/test_remote_register.py index cb037985cf..67d0f63b1f 100644 --- a/plugins/flytekit-spark/tests/test_remote_register.py +++ b/plugins/flytekit-spark/tests/test_remote_register.py @@ -12,6 +12,10 @@ def test_spark_template_with_remote(mock_insecure, mock_url): def my_spark(a: str) -> int: return 10 + @task + def my_python_task(a: str) -> int: + return 10 + mock_url.get.return_value = "localhost" mock_insecure.get.return_value = True @@ -25,6 +29,12 @@ def my_spark(a: str) -> int: remote.register(my_spark) serialized_spec = mock_client.create_task.call_args.kwargs["task_spec"] - # Check if the serialized task has mainApplicaitonFile field set. + # Check if the serialized spark task has mainApplicaitonFile field set. assert serialized_spec.template.custom["mainApplicationFile"] assert serialized_spec.template.custom["sparkConf"] + + remote.register(my_python_task) + serialized_spec = mock_client.create_task.call_args.kwargs["task_spec"] + + # Check if the serialized python task has no mainApplicaitonFile field set by default. + assert serialized_spec.template.custom is None From 85762ca5a63d1b3c9982765b46f96112d510a672 Mon Sep 17 00:00:00 2001 From: Yee Hing Tong Date: Mon, 15 Nov 2021 22:44:32 -0800 Subject: [PATCH 4/6] just adding an entrypoint setting using venv root for now, need to think about it more Signed-off-by: Yee Hing Tong --- flytekit/remote/remote.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/flytekit/remote/remote.py b/flytekit/remote/remote.py index 16c650e43d..945d7a4572 100644 --- a/flytekit/remote/remote.py +++ b/flytekit/remote/remote.py @@ -137,6 +137,7 @@ def from_config( default_domain: typing.Optional[str] = None, config_file_path: typing.Optional[str] = None, grpc_credentials: typing.Optional[grpc.ChannelCredentials] = None, + venv_root: typing.Optional[str] = None, ) -> FlyteRemote: """Create a FlyteRemote object using flyte configuration variables and/or environment variable overrides. @@ -160,6 +161,11 @@ def from_config( raw_output_prefix=raw_output_data_prefix, ) + venv_root = venv_root or serialize._DEFAULT_FLYTEKIT_VIRTUALENV_ROOT + entrypoint = context_manager.EntrypointSettings( + path=os.path.join(venv_root, serialize._DEFAULT_FLYTEKIT_RELATIVE_ENTRYPOINT_LOC) + ) + return cls( flyte_admin_url=platform_config.URL.get(), insecure=platform_config.INSECURE.get(), @@ -178,6 +184,7 @@ def from_config( admin_common_models.RawOutputDataConfig(raw_output_data_prefix) if raw_output_data_prefix else None ), grpc_credentials=grpc_credentials, + entrypoint_settings=entrypoint, ) def __init__( @@ -194,6 +201,7 @@ def __init__( image_config: typing.Optional[ImageConfig] = None, raw_output_data_config: typing.Optional[admin_common_models.RawOutputDataConfig] = None, grpc_credentials: typing.Optional[grpc.ChannelCredentials] = None, + entrypoint_settings: typing.Optional[context_manager.EntrypointSettings] = None, ): """Initialize a FlyteRemote object. @@ -226,6 +234,7 @@ def __init__( self._labels = labels self._annotations = annotations self._raw_output_data_config = raw_output_data_config + self._entrypoint_settings = entrypoint_settings # Save the file access object locally, but also make it available for use from the context. FlyteContextManager.with_context(FlyteContextManager.current_context().with_file_access(file_access).build()) From e63825569aa550e45287241d1d80e535be6b146b Mon Sep 17 00:00:00 2001 From: Yee Hing Tong Date: Wed, 17 Nov 2021 11:58:37 -0800 Subject: [PATCH 5/6] use entrypoint, add comment Signed-off-by: Yee Hing Tong --- flytekit/remote/remote.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/flytekit/remote/remote.py b/flytekit/remote/remote.py index 945d7a4572..0d7ee56c5e 100644 --- a/flytekit/remote/remote.py +++ b/flytekit/remote/remote.py @@ -216,7 +216,11 @@ def __init__( :param annotations: annotation config :param image_config: image config :param raw_output_data_config: location for offloaded data, e.g. in S3 - :param grpc_credentials: gRPC channel credentials for connecting to flyte admin as returned by :func:`grpc.ssl_channel_credentials` + :param grpc_credentials: gRPC channel credentials for connecting to flyte admin as returned + by :func:`grpc.ssl_channel_credentials` + :param entrypoint_settings: EntrypointSettings object for use with Spark tasks. If supplied, this will be + used when serializing Spark tasks, which need to know the path to the flytekit entrypoint.py file, + inside the container. """ remote_logger.warning("This feature is still in beta. Its interface and UX is subject to change.") if flyte_admin_url is None: @@ -234,6 +238,7 @@ def __init__( self._labels = labels self._annotations = annotations self._raw_output_data_config = raw_output_data_config + # Not exposing this as a property for now. self._entrypoint_settings = entrypoint_settings # Save the file access object locally, but also make it available for use from the context. @@ -543,11 +548,7 @@ def _serialize( self.image_config, # https://github.com/flyteorg/flyte/issues/1359 env={internal.IMAGE.env_var: self.image_config.default_image.full}, - entrypoint_settings=EntrypointSettings( - path=os.path.join( - serialize._DEFAULT_FLYTEKIT_VIRTUALENV_ROOT, serialize._DEFAULT_FLYTEKIT_RELATIVE_ENTRYPOINT_LOC - ) - ), + entrypoint_settings=self._entrypoint_settings, ), entity=entity, ) From 5d22cb79e6ee84efa93d0a46a0ad90e02e1eab0d Mon Sep 17 00:00:00 2001 From: Yee Hing Tong Date: Wed, 17 Nov 2021 12:09:10 -0800 Subject: [PATCH 6/6] nit Signed-off-by: Yee Hing Tong --- flytekit/remote/remote.py | 1 - 1 file changed, 1 deletion(-) diff --git a/flytekit/remote/remote.py b/flytekit/remote/remote.py index 0d7ee56c5e..76e8df1350 100644 --- a/flytekit/remote/remote.py +++ b/flytekit/remote/remote.py @@ -45,7 +45,6 @@ from flytekit.configuration.internal import DOMAIN, PROJECT from flytekit.core.base_task import PythonTask from flytekit.core.context_manager import ( - EntrypointSettings, FlyteContextManager, ImageConfig, SerializationSettings,