Make decks opt-in (#1251)

* turn off by default Signed-off-by: Yee Hing Tong <[email protected]> * update the config name Signed-off-by: Yee Hing Tong <[email protected]> * Make decks an opt-in feature at the task level Signed-off-by: Eduardo Apolinario <[email protected]> * Remove enable_deck Signed-off-by: Eduardo Apolinario <[email protected]> * Remove mention in docstring Signed-off-by: Eduardo Apolinario <[email protected]> * Linting Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Signed-off-by: Vivek Praharsha <[email protected]>
flyteorg · Oct 23, 2022 · bf95827 · bf95827
1 parent af70519
commit bf95827
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 22 deletions.
diff --git a/flytekit/configuration/internal.py b/flytekit/configuration/internal.py
@@ -35,11 +35,6 @@ def get_specified_images(cfg: typing.Optional[ConfigFile]) -> typing.Dict[str, s
             return cfg.yaml_config.get("images", images)
 
 
-class Deck(object):
-    SECTION = "deck"
-    DISABLE_DECK = ConfigEntry(LegacyConfigEntry(SECTION, "disable_deck", bool))
-
-
 class AWS(object):
     SECTION = "aws"
     S3_ENDPOINT = ConfigEntry(LegacyConfigEntry(SECTION, "endpoint"), YamlConfigEntry("storage.connection.endpoint"))

diff --git a/flytekit/core/base_task.py b/flytekit/core/base_task.py
@@ -24,7 +24,6 @@
 from typing import Any, Dict, Generic, List, Optional, OrderedDict, Tuple, Type, TypeVar, Union
 
 from flytekit.configuration import SerializationSettings
-from flytekit.configuration import internal as _internal
 from flytekit.core.context_manager import ExecutionParameters, FlyteContext, FlyteContextManager, FlyteEntities
 from flytekit.core.interface import Interface, transform_interface_to_typed_interface
 from flytekit.core.local_cache import LocalTaskCache
@@ -365,7 +364,7 @@ def __init__(
         task_config: T,
         interface: Optional[Interface] = None,
         environment: Optional[Dict[str, str]] = None,
-        disable_deck: bool = False,
+        disable_deck: bool = True,
         **kwargs,
     ):
         """
@@ -527,18 +526,18 @@ def dispatch_execute(
                         f"Failed to convert return value for var {k} for function {self.name} with error {type(e)}: {e}"
                     ) from e
 
-            INPUT = "input"
-            OUTPUT = "output"
+            if self._disable_deck is False:
+                INPUT = "input"
+                OUTPUT = "output"
 
-            input_deck = Deck(INPUT)
-            for k, v in native_inputs.items():
-                input_deck.append(TypeEngine.to_html(ctx, v, self.get_type_for_input_var(k, v)))
+                input_deck = Deck(INPUT)
+                for k, v in native_inputs.items():
+                    input_deck.append(TypeEngine.to_html(ctx, v, self.get_type_for_input_var(k, v)))
 
-            output_deck = Deck(OUTPUT)
-            for k, v in native_outputs_as_map.items():
-                output_deck.append(TypeEngine.to_html(ctx, v, self.get_type_for_output_var(k, v)))
+                output_deck = Deck(OUTPUT)
+                for k, v in native_outputs_as_map.items():
+                    output_deck.append(TypeEngine.to_html(ctx, v, self.get_type_for_output_var(k, v)))
 
-            if _internal.Deck.DISABLE_DECK.read() is not True and self.disable_deck is False:
                 _output_deck(self.name.split(".")[-1], new_user_params)
 
             outputs_literal_map = _literal_models.LiteralMap(literals=literals)

diff --git a/flytekit/core/task.py b/flytekit/core/task.py
@@ -89,7 +89,7 @@ def task(
     secret_requests: Optional[List[Secret]] = None,
     execution_mode: Optional[PythonFunctionTask.ExecutionBehavior] = PythonFunctionTask.ExecutionBehavior.DEFAULT,
     task_resolver: Optional[TaskResolverMixin] = None,
-    disable_deck: bool = False,
+    disable_deck: bool = True,
 ) -> Union[Callable, PythonFunctionTask]:
     """
     This is the core decorator to use for any task type in flytekit.

diff --git a/tests/flytekit/unit/core/test_flyte_file.py b/tests/flytekit/unit/core/test_flyte_file.py
@@ -164,7 +164,7 @@ def my_wf() -> FlyteFile:
         # This second layer should have two dirs, a random one generated by the new_execution_context call
         # and an empty folder, created by FlyteFile transformer's to_python_value function. This folder will have
         # something in it after we open() it.
-        assert len(working_dir) == 2
+        assert len(working_dir) == 1
 
         assert not os.path.exists(workflow_output.path)
         # # The act of opening it should trigger the download, since we do lazy downloading.
@@ -214,7 +214,7 @@ def my_wf() -> FlyteFile:
         # The act of running the workflow should create the engine dir, and the directory that will contain the
         # file but the file itself isn't downloaded yet.
         working_dir = os.listdir(os.path.join(random_dir, "local_flytekit"))
-        assert len(working_dir) == 2  # local flytekit and the downloaded file
+        assert len(working_dir) == 1  # local flytekit and the downloaded file
 
         assert not os.path.exists(workflow_output.path)
         # # The act of opening it should trigger the download, since we do lazy downloading.
@@ -224,7 +224,7 @@ def my_wf() -> FlyteFile:
         # and an empty folder, created by FlyteFile transformer's to_python_value function. This folder will have
         # something in it after we open() it.
         working_dir = os.listdir(os.path.join(random_dir, "local_flytekit"))
-        assert len(working_dir) == 3  # local flytekit and the downloaded file
+        assert len(working_dir) == 2  # local flytekit and the downloaded file
 
         assert os.path.exists(workflow_output.path)
 

diff --git a/tests/flytekit/unit/deck/test_deck.py b/tests/flytekit/unit/deck/test_deck.py
@@ -1,4 +1,5 @@
 import pandas as pd
+import pytest
 from mock import mock
 
 import flytekit
@@ -21,12 +22,53 @@ def test_deck():
 
     _output_deck("test_task", ctx.user_space_params)
 
-    @task()
+
+@pytest.mark.parametrize(
+    "disable_deck,expected_decks",
+    [
+        (None, 0),
+        (False, 2),  # input and output decks
+        (True, 0),
+    ],
+)
+def test_deck_for_task(disable_deck, expected_decks):
+    ctx = FlyteContextManager.current_context()
+
+    kwargs = {}
+    if disable_deck is not None:
+        kwargs["disable_deck"] = disable_deck
+
+    @task(**kwargs)
     def t1(a: int) -> str:
         return str(a)
 
     t1(a=3)
-    assert len(ctx.user_space_params.decks) == 2  # input, output decks
+    assert len(ctx.user_space_params.decks) == expected_decks
+
+
+@pytest.mark.parametrize(
+    "disable_deck, expected_decks",
+    [
+        (None, 1),
+        (False, 1 + 2),  # input and output decks
+        (True, 1),
+    ],
+)
+def test_deck_pandas_dataframe(disable_deck, expected_decks):
+    ctx = FlyteContextManager.current_context()
+
+    kwargs = {}
+    if disable_deck is not None:
+        kwargs["disable_deck"] = disable_deck
+
+    @task(**kwargs)
+    def t_df(a: str) -> int:
+        df = pd.DataFrame({"a": [1, 2, 3], "b": [4, 5, 6]})
+        flytekit.current_context().default_deck.append(TopFrameRenderer().to_html(df))
+        return int(a)
+
+    t_df(a="42")
+    assert len(ctx.user_space_params.decks) == expected_decks
 
 
 @mock.patch("flytekit.deck.deck._ipython_check")