From e3f33133a4454aee8807787988f62eb0ae3db8fd Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Tue, 21 Mar 2023 15:46:54 -0400 Subject: [PATCH 01/13] improve input type conversion error Signed-off-by: Niels Bantilan --- flytekit/__init__.py | 8 +++++++ flytekit/core/base_task.py | 41 +++++++++++++++++++++++------------ flytekit/core/promise.py | 2 +- flytekit/core/type_engine.py | 12 +++++++--- flytekit/exceptions/scopes.py | 7 ++++-- flytekit/loggers.py | 24 +++++++++++++++----- flytekit/models/common.py | 3 ++- type_error_conversion.py | 21 ++++++++++++++++++ 8 files changed, 92 insertions(+), 26 deletions(-) create mode 100644 type_error_conversion.py diff --git a/flytekit/__init__.py b/flytekit/__init__.py index c2fc11816c..6a8070ad28 100644 --- a/flytekit/__init__.py +++ b/flytekit/__init__.py @@ -194,6 +194,7 @@ import sys from typing import Generator + if sys.version_info < (3, 10): from importlib_metadata import entry_points else: @@ -296,3 +297,10 @@ def load_implicit_plugins(): # Load all implicit plugins load_implicit_plugins() + +# Pretty-print exception messages +try: + from rich import traceback + traceback.install() +except ImportError: + pass diff --git a/flytekit/core/base_task.py b/flytekit/core/base_task.py index f163e891e1..6ef5cbb1bf 100644 --- a/flytekit/core/base_task.py +++ b/flytekit/core/base_task.py @@ -43,7 +43,7 @@ translate_inputs_to_literals, ) from flytekit.core.tracker import TrackedInstance -from flytekit.core.type_engine import TypeEngine +from flytekit.core.type_engine import TypeEngine, TypeTransformerFailedError from flytekit.deck.deck import Deck from flytekit.loggers import logger from flytekit.models import dynamic_job as _dynamic_job @@ -245,12 +245,17 @@ def local_execute(self, ctx: FlyteContext, **kwargs) -> Union[Tuple[Promise], Pr # Promises as essentially inputs from previous task executions # native constants are just bound to this specific task (default values for a task input) # Also along with promises and constants, there could be dictionary or list of promises or constants - kwargs = translate_inputs_to_literals( - ctx, - incoming_values=kwargs, - flyte_interface_types=self.interface.inputs, - native_types=self.get_input_types(), # type: ignore - ) + try: + kwargs = translate_inputs_to_literals( + ctx, + incoming_values=kwargs, + flyte_interface_types=self.interface.inputs, + native_types=self.get_input_types(), # type: ignore + ) + except TypeTransformerFailedError as exc: + msg = f"Failed to convert inputs in task '{self.name}':\n {exc}" + logger.error(msg) + raise TypeError(msg) from exc input_literal_map = _literal_models.LiteralMap(literals=kwargs) # if metadata.cache is set, check memoized version @@ -515,7 +520,14 @@ def dispatch_execute( ) as exec_ctx: # TODO We could support default values here too - but not part of the plan right now # Translate the input literals to Python native - native_inputs = TypeEngine.literal_map_to_kwargs(exec_ctx, input_literal_map, self.python_interface.inputs) + try: + native_inputs = TypeEngine.literal_map_to_kwargs( + exec_ctx, input_literal_map, self.python_interface.inputs + ) + except Exception as exc: + msg = f"Failed to convert inputs in task '{self.name}':\n {exc}" + logger.error(msg) + raise TypeError(msg) from exc # TODO: Logger should auto inject the current context information to indicate if the task is running within # a workflow or a subworkflow etc @@ -559,19 +571,20 @@ def dispatch_execute( # We manually construct a LiteralMap here because task inputs and outputs actually violate the assumption # built into the IDL that all the values of a literal map are of the same type. literals = {} - for k, v in native_outputs_as_map.items(): + for i, (k, v) in enumerate(native_outputs_as_map.items()): literal_type = self._outputs_interface[k].type py_type = self.get_type_for_output_var(k, v) if isinstance(v, tuple): - raise TypeError(f"Output({k}) in task{self.name} received a tuple {v}, instead of {py_type}") + raise TypeError(f"Output({k}) in task '{self.name}' received a tuple {v}, instead of {py_type}") try: literals[k] = TypeEngine.to_literal(exec_ctx, v, py_type, literal_type) except Exception as e: - logger.error(f"Failed to convert return value for var {k} with error {type(e)}: {e}") - raise TypeError( - f"Failed to convert return value for var {k} for function {self.name} with error {type(e)}: {e}" - ) from e + # only show the name of output key if it's user-defined (by default Flyte names these as "o") + key = k if k != f"o{i}" else i + msg = f"Failed to convert outputs of task '{self.name}' at position {key}:\n {e}" + logger.error(msg) + raise TypeError(msg) from e if self._disable_deck is False: INPUT = "input" diff --git a/flytekit/core/promise.py b/flytekit/core/promise.py index 8be9d8ccae..84e65e6ac4 100644 --- a/flytekit/core/promise.py +++ b/flytekit/core/promise.py @@ -1057,7 +1057,7 @@ def flyte_entity_call_handler( for k, v in kwargs.items(): if k not in cast(SupportsNodeCreation, entity).python_interface.inputs: raise ValueError( - f"Received unexpected keyword argument {k} in function {cast(SupportsNodeCreation, entity).name}" + f"Received unexpected keyword argument '{k}' in function '{cast(SupportsNodeCreation, entity).name}'" ) ctx = FlyteContextManager.current_context() diff --git a/flytekit/core/type_engine.py b/flytekit/core/type_engine.py index 306c4116ad..488c0680cd 100644 --- a/flytekit/core/type_engine.py +++ b/flytekit/core/type_engine.py @@ -88,7 +88,7 @@ def type_assertions_enabled(self) -> bool: def assert_type(self, t: Type[T], v: T): if not hasattr(t, "__origin__") and not isinstance(v, t): - raise TypeTransformerFailedError(f"Type of Val '{v}' is not an instance of {t}") + raise TypeTransformerFailedError(f"Value '{v}' of type {type(v)} is not an instance of {t}") @abstractmethod def get_literal_type(self, t: Type[T]) -> LiteralType: @@ -185,7 +185,7 @@ def to_python_value(self, ctx: FlyteContext, lv: Literal, expected_python_type: return res except AttributeError: # Assume that this is because a property on `lv` was None - raise TypeTransformerFailedError(f"Cannot convert literal {lv}") + raise TypeTransformerFailedError(f"Cannot convert literal {lv} to {self._type}") def guess_python_type(self, literal_type: LiteralType) -> Type[T]: if literal_type.simple is not None and literal_type.simple == self._lt.simple: @@ -864,7 +864,13 @@ def literal_map_to_kwargs( raise ValueError( f"Received more input values {len(lm.literals)}" f" than allowed by the input spec {len(python_types)}" ) - return {k: TypeEngine.to_python_value(ctx, lm.literals[k], python_types[k]) for k, v in lm.literals.items()} + kwargs = {} + for i, k in enumerate(lm.literals): + try: + kwargs[k] = TypeEngine.to_python_value(ctx, lm.literals[k], python_types[k]) + except TypeTransformerFailedError as exc: + raise TypeTransformerFailedError(f"Error converting input '{k}' at position {i}:\n {exc}") from exc + return kwargs @classmethod def dict_to_literal_map( diff --git a/flytekit/exceptions/scopes.py b/flytekit/exceptions/scopes.py index 60a4afa97e..bdfb2ba182 100644 --- a/flytekit/exceptions/scopes.py +++ b/flytekit/exceptions/scopes.py @@ -194,10 +194,13 @@ def user_entry_point(wrapped, instance, args, kwargs): _CONTEXT_STACK.append(_USER_CONTEXT) if _is_base_context(): # See comment at this location for system_entry_point + fn_name = wrapped.__name__ try: return wrapped(*args, **kwargs) - except FlyteScopedException as ex: - raise ex.value + except FlyteScopedException as exc: + raise exc.type(f"Error encountered while executing '{fn_name}':\n {exc.value}") from exc + except Exception as exc: + raise type(exc)(f"Error encountered while executing '{fn_name}':\n {exc}") from exc else: try: return wrapped(*args, **kwargs) diff --git a/flytekit/loggers.py b/flytekit/loggers.py index f047348de0..47aeaa82cd 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -10,6 +10,7 @@ # For now, assume this is the environment variable whose usage will remain unchanged and controls output for all # loggers defined in this file. LOGGING_ENV_VAR = "FLYTE_SDK_LOGGING_LEVEL" +LOGGING_FMT_ENV_VAR = "FLYTE_SDK_LOGGING_FORMAT" # By default, the root flytekit logger to debug so everything is logged, but enable fine-tuning logger = logging.getLogger("flytekit") @@ -33,8 +34,17 @@ user_space_logger = child_loggers["user_space"] # create console handler -ch = logging.StreamHandler() -ch.setLevel(logging.DEBUG) +try: + from rich.logging import RichHandler + handler = RichHandler( + rich_tracebacks=True, + omit_repeated_times=False, + keywords=["[flytekit]"] + ) +except ImportError: + handler = logging.StreamHandler() + +handler.setLevel(logging.DEBUG) # Root logger control # Don't want to import the configuration library since that will cause all sorts of circular imports, let's @@ -63,10 +73,14 @@ child_logger.setLevel(logging.WARNING) # create formatter -formatter = jsonlogger.JsonFormatter(fmt="%(asctime)s %(name)s %(levelname)s %(message)s") +logging_fmt = os.environ.get(LOGGING_FMT_ENV_VAR, "") +if logging_fmt == "json": + formatter = jsonlogger.JsonFormatter(fmt="%(asctime)s %(name)s %(levelname)s %(message)s") +else: + formatter = logging.Formatter(fmt="[%(name)s] %(message)s") # add formatter to ch -ch.setFormatter(formatter) +handler.setFormatter(formatter) # add ch to logger -logger.addHandler(ch) +logger.addHandler(handler) diff --git a/flytekit/models/common.py b/flytekit/models/common.py index 62018c1eef..2c52b14a0e 100644 --- a/flytekit/models/common.py +++ b/flytekit/models/common.py @@ -57,7 +57,8 @@ def short_string(self): """ :rtype: Text """ - return str(self.to_flyte_idl()) + out = str(self.to_flyte_idl()).replace("\n", "").replace(" " * 4, "").replace(" " * 2, "") + return f"" def verbose_string(self): """ diff --git a/type_error_conversion.py b/type_error_conversion.py new file mode 100644 index 0000000000..95f28e0d91 --- /dev/null +++ b/type_error_conversion.py @@ -0,0 +1,21 @@ +import random +from flytekit import task, workflow + + +@task +def add_rand(n: int) -> float: + return float(n + random.randint(-1000, 1000)) + +@task +def bad_types(a: int) -> float: + return str(a) + +@task +def wf(a: int, b: int): + bad_types(a=add_rand(n=a)) + + +if __name__ == "__main__": + print(wf(a=1, b=1)) + # print(bad_types(a=1)) + # print(bad_types(a=str(1))) From 36c9af2040385749b1012000251b3e1a40e1991e Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Wed, 5 Apr 2023 13:52:13 -0400 Subject: [PATCH 02/13] fix lint Signed-off-by: Niels Bantilan --- flytekit/__init__.py | 2 +- flytekit/loggers.py | 7 +-- flytekit/models/common.py | 5 +- .../core/flyte_functools/test_decorators.py | 2 +- tests/flytekit/unit/core/test_promise.py | 6 +-- tests/flytekit/unit/core/test_type_hints.py | 48 +++++++++---------- type_error_conversion.py | 9 ++-- 7 files changed, 38 insertions(+), 41 deletions(-) diff --git a/flytekit/__init__.py b/flytekit/__init__.py index 6a8070ad28..9078979e59 100644 --- a/flytekit/__init__.py +++ b/flytekit/__init__.py @@ -194,7 +194,6 @@ import sys from typing import Generator - if sys.version_info < (3, 10): from importlib_metadata import entry_points else: @@ -301,6 +300,7 @@ def load_implicit_plugins(): # Pretty-print exception messages try: from rich import traceback + traceback.install() except ImportError: pass diff --git a/flytekit/loggers.py b/flytekit/loggers.py index 47aeaa82cd..a7cfcc04b7 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -36,11 +36,8 @@ # create console handler try: from rich.logging import RichHandler - handler = RichHandler( - rich_tracebacks=True, - omit_repeated_times=False, - keywords=["[flytekit]"] - ) + + handler = RichHandler(rich_tracebacks=True, omit_repeated_times=False, keywords=["[flytekit]"]) except ImportError: handler = logging.StreamHandler() diff --git a/flytekit/models/common.py b/flytekit/models/common.py index 2c52b14a0e..4f030e25a4 100644 --- a/flytekit/models/common.py +++ b/flytekit/models/common.py @@ -1,5 +1,6 @@ import abc as _abc import json as _json +import re from flyteidl.admin import common_pb2 as _common_pb2 from google.protobuf import json_format as _json_format @@ -57,8 +58,8 @@ def short_string(self): """ :rtype: Text """ - out = str(self.to_flyte_idl()).replace("\n", "").replace(" " * 4, "").replace(" " * 2, "") - return f"" + literal_str = re.sub(r"\s+", " ", str(self.to_flyte_idl())).strip() + return f"" def verbose_string(self): """ diff --git a/tests/flytekit/unit/core/flyte_functools/test_decorators.py b/tests/flytekit/unit/core/flyte_functools/test_decorators.py index 3edd547c8a..d5c0e356db 100644 --- a/tests/flytekit/unit/core/flyte_functools/test_decorators.py +++ b/tests/flytekit/unit/core/flyte_functools/test_decorators.py @@ -39,7 +39,7 @@ def test_wrapped_tasks_error(capfd): ) out = capfd.readouterr().out - assert out.replace("\r", "").strip().split("\n") == [ + assert out.replace("\r", "").strip().split("\n")[:5] == [ "before running my_task", "try running my_task", "error running my_task: my_task failed with input: 0", diff --git a/tests/flytekit/unit/core/test_promise.py b/tests/flytekit/unit/core/test_promise.py index d8b043116e..9bb100cd6b 100644 --- a/tests/flytekit/unit/core/test_promise.py +++ b/tests/flytekit/unit/core/test_promise.py @@ -74,7 +74,7 @@ def wf(i: int, j: int): # without providing the _inputs_not_allowed or _ignorable_inputs, all inputs to lp become required, # which is incorrect - with pytest.raises(FlyteAssertion, match="Missing input `i` type `simple: INTEGER"): + with pytest.raises(FlyteAssertion, match="Missing input `i` type ``"): create_and_link_node_from_remote(ctx, lp) # Even if j is not provided it will default @@ -111,7 +111,7 @@ def t1(a: typing.Union[float, typing.List[int], MyDataclass]): def test_translate_inputs_to_literals_with_wrong_types(): ctx = context_manager.FlyteContext.current_context() - with pytest.raises(TypeError, match="Not a map type union_type"): + with pytest.raises(TypeError, match="Not a map type typing.Tuple[int, str]: def foo3(a: typing.Dict) -> typing.Dict: return a - with pytest.raises(TypeError, match="Type of Val 'hello' is not an instance of "): + with pytest.raises( + TypeError, + match=( + "Failed to convert inputs in task 'tests.flytekit.unit.core.test_type_hints.foo':\n " + "Value 'hello' of type is not an instance of " + ), + ): foo(a="hello", b=10) # type: ignore with pytest.raises( TypeError, - match="Failed to convert return value for var o0 for " "function tests.flytekit.unit.core.test_type_hints.foo2", + match=( + "Failed to convert outputs of task 'tests.flytekit.unit.core.test_type_hints.foo2' at position 0:\n " + "Value 'hello' of type is not an instance of " + ), ): foo2(a=10, b="hello") - with pytest.raises(TypeError, match="Not a collection type simple: STRUCT\n but got a list \\[{'hello': 2}\\]"): + with pytest.raises( + TypeError, + match="Not a collection type but got a list \\[{'hello': 2}\\]", + ): foo3(a=[{"hello": 2}]) # type: ignore @@ -1672,28 +1684,12 @@ def wf2(a: typing.Union[int, str]) -> typing.Union[int, str]: with pytest.raises( TypeError, - match=dedent( - r""" - Cannot convert from scalar { - union { - value { - scalar { - primitive { - string_value: "2" - } - } - } - type { - simple: STRING - structure { - tag: "str" - } - } - } - } - to typing.Union\[float, dict\] \(using tag str\) - """ - )[1:-1], + match=re.escape( + "Error encountered while executing 'wf2':\n " + "Failed to convert inputs in task 'tests.flytekit.unit.core.test_type_hints.t2':\n " + 'Cannot convert from to typing.Union[float, dict] (using tag str)' + ), ): assert wf2(a="2") == "2" diff --git a/type_error_conversion.py b/type_error_conversion.py index 95f28e0d91..799ec0e4ba 100644 --- a/type_error_conversion.py +++ b/type_error_conversion.py @@ -1,19 +1,22 @@ import random + from flytekit import task, workflow -@task +@task def add_rand(n: int) -> float: return float(n + random.randint(-1000, 1000)) + @task def bad_types(a: int) -> float: return str(a) -@task + +@workflow def wf(a: int, b: int): bad_types(a=add_rand(n=a)) - + if __name__ == "__main__": print(wf(a=1, b=1)) From e0ecea882d93d0fe70f6f2b07a539e0a58fe8163 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Wed, 5 Apr 2023 14:44:20 -0400 Subject: [PATCH 03/13] fix Signed-off-by: Niels Bantilan --- flytekit/__init__.py | 2 +- .../unit/core/flyte_functools/test_decorators.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/flytekit/__init__.py b/flytekit/__init__.py index 9078979e59..a1574001cb 100644 --- a/flytekit/__init__.py +++ b/flytekit/__init__.py @@ -301,6 +301,6 @@ def load_implicit_plugins(): try: from rich import traceback - traceback.install() + traceback.install(width=None) except ImportError: pass diff --git a/tests/flytekit/unit/core/flyte_functools/test_decorators.py b/tests/flytekit/unit/core/flyte_functools/test_decorators.py index d5c0e356db..20e55e9d3c 100644 --- a/tests/flytekit/unit/core/flyte_functools/test_decorators.py +++ b/tests/flytekit/unit/core/flyte_functools/test_decorators.py @@ -74,11 +74,11 @@ def test_unwrapped_task(): capture_output=True, ) error = completed_process.stderr - error_str = error.strip().split("\n")[-1] - assert ( - "TaskFunction cannot be a nested/inner or local function." - " It should be accessible at a module level for Flyte to execute it." in error_str - ) + error_str = "" + for line in error.strip().split("\n"): + if line.startswith("ValueError"): + error_str += line + assert error_str.startswith("ValueError: TaskFunction cannot be a nested/inner or local function.") @pytest.mark.parametrize("script", ["nested_function.py", "nested_wrapped_function.py"]) @@ -90,5 +90,8 @@ def test_nested_function(script): capture_output=True, ) error = completed_process.stderr - error_str = error.strip().split("\n")[-1] + error_str = "" + for line in error.strip().split("\n"): + if line.startswith("ValueError"): + error_str += line assert error_str.startswith("ValueError: TaskFunction cannot be a nested/inner or local function.") From 70c96753153119fdb691be2d0c790203b051b46b Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Wed, 5 Apr 2023 16:57:15 -0400 Subject: [PATCH 04/13] add tests Signed-off-by: Niels Bantilan --- flytekit/__init__.py | 2 +- flytekit/core/promise.py | 12 +++++-- flytekit/core/workflow.py | 12 +++++-- flytekit/loggers.py | 6 ++-- .../unit/core/test_type_conversion_errors.py | 1 + type_error_conversion.py | 35 +++++++++++++++++-- 6 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 tests/flytekit/unit/core/test_type_conversion_errors.py diff --git a/flytekit/__init__.py b/flytekit/__init__.py index a1574001cb..d716b26716 100644 --- a/flytekit/__init__.py +++ b/flytekit/__init__.py @@ -301,6 +301,6 @@ def load_implicit_plugins(): try: from rich import traceback - traceback.install(width=None) + traceback.install(width=None, extra_lines=0) except ImportError: pass diff --git a/flytekit/core/promise.py b/flytekit/core/promise.py index 84e65e6ac4..e30141df02 100644 --- a/flytekit/core/promise.py +++ b/flytekit/core/promise.py @@ -477,10 +477,14 @@ def create_native_named_tuple( if isinstance(promises, Promise): k, v = [(k, v) for k, v in entity_interface.outputs.items()][0] # get output native type + # only show the name of output key if it's user-defined (by default Flyte names these as "o") + key = k if k != "o0" else 0 try: return TypeEngine.to_python_value(ctx, promises.val, v) except Exception as e: - raise AssertionError(f"Failed to convert value of output {k}, expected type {v}.") from e + raise TypeError( + f"Failed to convert output in position {key} of value {promises.val}, expected type {v}." + ) from e if len(cast(Tuple[Promise], promises)) == 0: return None @@ -490,7 +494,7 @@ def create_native_named_tuple( named_tuple_name = entity_interface.output_tuple_name outputs = {} - for p in cast(Tuple[Promise], promises): + for i, p in enumerate(cast(Tuple[Promise], promises)): if not isinstance(p, Promise): raise AssertionError( "Workflow outputs can only be promises that are returned by tasks. Found a value of" @@ -500,7 +504,9 @@ def create_native_named_tuple( try: outputs[p.var] = TypeEngine.to_python_value(ctx, p.val, t) except Exception as e: - raise AssertionError(f"Failed to convert value of output {p.var}, expected type {t}.") from e + # only show the name of output key if it's user-defined (by default Flyte names these as "o") + key = p.var if p.var != f"o{i}" else i + raise TypeError(f"Failed to convert output in position {key} of value {p.val}, expected type {t}.") from e # Should this class be part of the Interface? nt = collections.namedtuple(named_tuple_name, list(outputs.keys())) # type: ignore diff --git a/flytekit/core/workflow.py b/flytekit/core/workflow.py index b716eb7114..2c0a058c0e 100644 --- a/flytekit/core/workflow.py +++ b/flytekit/core/workflow.py @@ -32,7 +32,7 @@ from flytekit.core.python_auto_container import PythonAutoContainerTask from flytekit.core.reference_entity import ReferenceEntity, WorkflowReference from flytekit.core.tracker import extract_task_module -from flytekit.core.type_engine import TypeEngine +from flytekit.core.type_engine import TypeEngine, TypeTransformerFailedError from flytekit.exceptions import scopes as exception_scopes from flytekit.exceptions.user import FlyteValidationException, FlyteValueException from flytekit.loggers import logger @@ -260,7 +260,10 @@ def __call__(self, *args, **kwargs) -> Union[Tuple[Promise], Promise, VoidPromis input_kwargs = self.python_interface.default_inputs_as_kwargs input_kwargs.update(kwargs) self.compile() - return flyte_entity_call_handler(self, *args, **input_kwargs) + try: + return flyte_entity_call_handler(self, *args, **input_kwargs) + except TypeError as exc: + raise TypeError(f"Failed to convert outputs in workflow '{self.name}':\n {exc}") from exc def execute(self, **kwargs): raise Exception("Should not be called") @@ -274,7 +277,10 @@ def local_execute(self, ctx: FlyteContext, **kwargs) -> Union[Tuple[Promise], Pr for k, v in kwargs.items(): if not isinstance(v, Promise): t = self.python_interface.inputs[k] - kwargs[k] = Promise(var=k, val=TypeEngine.to_literal(ctx, v, t, self.interface.inputs[k].type)) + try: + kwargs[k] = Promise(var=k, val=TypeEngine.to_literal(ctx, v, t, self.interface.inputs[k].type)) + except TypeTransformerFailedError as exc: + raise TypeError(f"Failed to convert inputs in workflow '{self.name}':\n {exc}") from exc # The output of this will always be a combination of Python native values and Promises containing Flyte # Literals. diff --git a/flytekit/loggers.py b/flytekit/loggers.py index a7cfcc04b7..5b651b7d6f 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -37,7 +37,9 @@ try: from rich.logging import RichHandler - handler = RichHandler(rich_tracebacks=True, omit_repeated_times=False, keywords=["[flytekit]"]) + handler = RichHandler( + rich_tracebacks=True, omit_repeated_times=False, keywords=["[flytekit]"], log_time_format="%Y-%m-%d %H:%M:%S,%f" + ) except ImportError: handler = logging.StreamHandler() @@ -76,7 +78,7 @@ else: formatter = logging.Formatter(fmt="[%(name)s] %(message)s") -# add formatter to ch +# add formatter to the handler handler.setFormatter(formatter) # add ch to logger diff --git a/tests/flytekit/unit/core/test_type_conversion_errors.py b/tests/flytekit/unit/core/test_type_conversion_errors.py new file mode 100644 index 0000000000..1d7699a317 --- /dev/null +++ b/tests/flytekit/unit/core/test_type_conversion_errors.py @@ -0,0 +1 @@ +"""Unit tests for type conversion errors.""" diff --git a/type_error_conversion.py b/type_error_conversion.py index 799ec0e4ba..9334ec5843 100644 --- a/type_error_conversion.py +++ b/type_error_conversion.py @@ -1,4 +1,5 @@ import random +from typing import Tuple from flytekit import task, workflow @@ -13,12 +14,40 @@ def bad_types(a: int) -> float: return str(a) +@task +def good_types(a: float) -> str: + return str(a) + + @workflow def wf(a: int, b: int): bad_types(a=add_rand(n=a)) +@workflow +def wf_bad_output(a: int, b: int) -> int: + return good_types(a=add_rand(n=a)) + + +@workflow +def wf_bad_multioutput1(a: int, b: int) -> Tuple[int, str]: + out1 = good_types(a=add_rand(n=a)) + out2 = good_types(a=add_rand(n=b)) + return out1, out2 + + +@workflow +def wf_bad_multioutput2(a: int, b: int) -> Tuple[str, int]: + out1 = good_types(a=add_rand(n=a)) + out2 = good_types(a=add_rand(n=b)) + return out1, out2 + + if __name__ == "__main__": - print(wf(a=1, b=1)) - # print(bad_types(a=1)) - # print(bad_types(a=str(1))) + # wf(a=1, b=1) + # wf(a=1, b=1.0) + # wf_bad_output(a=1, b=1) + # wf_bad_multioutput(a=1, b=2) + # wf_bad_multioutput2(a=1, b=2) + bad_types(a=1) + # bad_types(a=str(1)) From ec38ddeac05db85456133355fd23987face77e25 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 15:34:35 -0400 Subject: [PATCH 05/13] add tests Signed-off-by: Niels Bantilan --- .gitignore | 1 + dev-requirements.in | 1 + flytekit/core/base_task.py | 4 +- flytekit/core/promise.py | 8 +- flytekit/core/type_engine.py | 6 +- flytekit/core/workflow.py | 6 +- flytekit/loggers.py | 19 +-- .../unit/core/test_type_conversion_errors.py | 129 ++++++++++++++++++ tests/flytekit/unit/core/test_type_hints.py | 14 +- 9 files changed, 164 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index a4fe02503e..b2e20249a8 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,4 @@ htmlcov *.ipynb *dat docs/source/_tags/ +.hypothesis diff --git a/dev-requirements.in b/dev-requirements.in index 78cea49c61..5bb86ba612 100644 --- a/dev-requirements.in +++ b/dev-requirements.in @@ -2,6 +2,7 @@ git+https://github.com/flyteorg/pytest-flyte@main#egg=pytest-flyte coverage[toml] +hypothesis joblib mock pytest diff --git a/flytekit/core/base_task.py b/flytekit/core/base_task.py index 6ef5cbb1bf..c2ec5f0190 100644 --- a/flytekit/core/base_task.py +++ b/flytekit/core/base_task.py @@ -253,7 +253,7 @@ def local_execute(self, ctx: FlyteContext, **kwargs) -> Union[Tuple[Promise], Pr native_types=self.get_input_types(), # type: ignore ) except TypeTransformerFailedError as exc: - msg = f"Failed to convert inputs in task '{self.name}':\n {exc}" + msg = f"Failed to convert inputs of task '{self.name}':\n {exc}" logger.error(msg) raise TypeError(msg) from exc input_literal_map = _literal_models.LiteralMap(literals=kwargs) @@ -525,7 +525,7 @@ def dispatch_execute( exec_ctx, input_literal_map, self.python_interface.inputs ) except Exception as exc: - msg = f"Failed to convert inputs in task '{self.name}':\n {exc}" + msg = f"Failed to convert inputs of task '{self.name}':\n {exc}" logger.error(msg) raise TypeError(msg) from exc diff --git a/flytekit/core/promise.py b/flytekit/core/promise.py index e30141df02..2b1be4e9b6 100644 --- a/flytekit/core/promise.py +++ b/flytekit/core/promise.py @@ -19,7 +19,7 @@ ) from flytekit.core.interface import Interface from flytekit.core.node import Node -from flytekit.core.type_engine import DictTransformer, ListTransformer, TypeEngine +from flytekit.core.type_engine import DictTransformer, ListTransformer, TypeEngine, TypeTransformerFailedError from flytekit.exceptions import user as _user_exceptions from flytekit.models import interface as _interface_models from flytekit.models import literals as _literal_models @@ -141,7 +141,11 @@ def extract_value( raise ValueError(f"Received unexpected keyword argument {k}") var = flyte_interface_types[k] t = native_types[k] - result[k] = extract_value(ctx, v, t, var.type) + try: + result[k] = extract_value(ctx, v, t, var.type) + except TypeTransformerFailedError as exc: + raise TypeTransformerFailedError(f"Failed argument '{k}': {exc}") from exc + return result diff --git a/flytekit/core/type_engine.py b/flytekit/core/type_engine.py index 488c0680cd..2229999930 100644 --- a/flytekit/core/type_engine.py +++ b/flytekit/core/type_engine.py @@ -88,7 +88,7 @@ def type_assertions_enabled(self) -> bool: def assert_type(self, t: Type[T], v: T): if not hasattr(t, "__origin__") and not isinstance(v, t): - raise TypeTransformerFailedError(f"Value '{v}' of type {type(v)} is not an instance of {t}") + raise TypeTransformerFailedError(f"Expected value of type {t} but got '{v}' of type {type(v)}") @abstractmethod def get_literal_type(self, t: Type[T]) -> LiteralType: @@ -166,7 +166,9 @@ def get_literal_type(self, t: Optional[Type[T]] = None) -> LiteralType: def to_literal(self, ctx: FlyteContext, python_val: T, python_type: Type[T], expected: LiteralType) -> Literal: if type(python_val) != self._type: - raise TypeTransformerFailedError(f"Expected value of type {self._type} but got type {type(python_val)}") + raise TypeTransformerFailedError( + f"Expected value of type {self._type} but got '{python_val}' of type {type(python_val)}" + ) return self._to_literal_transformer(python_val) def to_python_value(self, ctx: FlyteContext, lv: Literal, expected_python_type: Type[T]) -> T: diff --git a/flytekit/core/workflow.py b/flytekit/core/workflow.py index 2c0a058c0e..da34b98229 100644 --- a/flytekit/core/workflow.py +++ b/flytekit/core/workflow.py @@ -263,7 +263,7 @@ def __call__(self, *args, **kwargs) -> Union[Tuple[Promise], Promise, VoidPromis try: return flyte_entity_call_handler(self, *args, **input_kwargs) except TypeError as exc: - raise TypeError(f"Failed to convert outputs in workflow '{self.name}':\n {exc}") from exc + raise TypeError(f"Encountered error while executing workflow '{self.name}':\n {exc}") from exc def execute(self, **kwargs): raise Exception("Should not be called") @@ -280,7 +280,9 @@ def local_execute(self, ctx: FlyteContext, **kwargs) -> Union[Tuple[Promise], Pr try: kwargs[k] = Promise(var=k, val=TypeEngine.to_literal(ctx, v, t, self.interface.inputs[k].type)) except TypeTransformerFailedError as exc: - raise TypeError(f"Failed to convert inputs in workflow '{self.name}':\n {exc}") from exc + raise TypeError( + f"Failed to convert input argument '{k}' of workflow '{self.name}':\n {exc}" + ) from exc # The output of this will always be a combination of Python native values and Promises containing Flyte # Literals. diff --git a/flytekit/loggers.py b/flytekit/loggers.py index 5b651b7d6f..79481e9a71 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -2,6 +2,8 @@ import os from pythonjsonlogger import jsonlogger +from rich.console import Console +from rich.logging import RichHandler # Note: # The environment variable controls exposed to affect the individual loggers should be considered to be beta. @@ -34,14 +36,13 @@ user_space_logger = child_loggers["user_space"] # create console handler -try: - from rich.logging import RichHandler - - handler = RichHandler( - rich_tracebacks=True, omit_repeated_times=False, keywords=["[flytekit]"], log_time_format="%Y-%m-%d %H:%M:%S,%f" - ) -except ImportError: - handler = logging.StreamHandler() +handler = RichHandler( + rich_tracebacks=True, + omit_repeated_times=False, + keywords=["[flytekit]"], + log_time_format="%Y-%m-%d %H:%M:%S,%f", + console=Console(width=os.get_terminal_size().columns), +) handler.setLevel(logging.DEBUG) @@ -72,7 +73,7 @@ child_logger.setLevel(logging.WARNING) # create formatter -logging_fmt = os.environ.get(LOGGING_FMT_ENV_VAR, "") +logging_fmt = os.environ.get(LOGGING_FMT_ENV_VAR, "json") if logging_fmt == "json": formatter = jsonlogger.JsonFormatter(fmt="%(asctime)s %(name)s %(levelname)s %(message)s") else: diff --git a/tests/flytekit/unit/core/test_type_conversion_errors.py b/tests/flytekit/unit/core/test_type_conversion_errors.py index 1d7699a317..71227b9f62 100644 --- a/tests/flytekit/unit/core/test_type_conversion_errors.py +++ b/tests/flytekit/unit/core/test_type_conversion_errors.py @@ -1 +1,130 @@ """Unit tests for type conversion errors.""" + +from string import ascii_lowercase +from typing import Tuple + +import pytest +from hypothesis import given, strategies as st + +from flytekit import task, workflow + + +@task +def int_to_float(n: int) -> float: + return float(n) + + +@task +def task_incorrect_output(a: float) -> int: + return str(a) + + +@task +def task_correct_output(a: float) -> str: + return str(a) + + +@workflow +def wf_with_task_error(a: int) -> str: + return task_incorrect_output(a=int_to_float(n=a)) + + +@workflow +def wf_with_output_error(a: int) -> int: + return task_correct_output(a=int_to_float(n=a)) + + +@workflow +def wf_with_multioutput_error0(a: int, b: int) -> Tuple[int, str]: + out_a = task_correct_output(a=int_to_float(n=a)) + out_b = task_correct_output(a=int_to_float(n=b)) + return out_a, out_b + + +@workflow +def wf_with_multioutput_error1(a: int, b: int) -> Tuple[str, int]: + out_a = task_correct_output(a=int_to_float(n=a)) + out_b = task_correct_output(a=int_to_float(n=b)) + return out_a, out_b + + +@given(st.booleans() | st.integers() | st.text(ascii_lowercase)) +def test_task_input_error(incorrect_input): + with pytest.raises( + TypeError, + match=( + r"Failed to convert inputs of task '{}':\n" + r" Failed argument 'a': Expected value of type \ but got .+ of type .+" + ).format(task_correct_output.name) + ): + task_correct_output(a=incorrect_input) + + +@given(st.floats()) +def test_task_output_error(correct_input): + with pytest.raises( + TypeError, + match=( + r"Failed to convert outputs of task '{}' at position 0:\n" + r" Expected value of type \ but got .+ of type .+" + ).format(task_incorrect_output.name) + ): + task_incorrect_output(a=correct_input) + + +@given(st.integers()) +def test_workflow_with_task_error(correct_input): + with pytest.raises( + TypeError, + match=( + r"Encountered error while executing workflow '{}':\n" + r" Error encountered while executing 'wf_with_task_error':\n" + r" Failed to convert outputs of task '.+' at position 0:\n" + r" Expected value of type \ but got .+ of type .+" + ).format(wf_with_task_error.name) + ): + wf_with_task_error(a=correct_input) + + +@given(st.booleans() | st.floats() | st.text(ascii_lowercase)) +def test_workflow_with_input_error(incorrect_input): + with pytest.raises( + TypeError, + match=( + r"Encountered error while executing workflow '{}':\n" + r" Failed to convert input argument 'a' of workflow '.+':\n" + r" Expected value of type \ but got .+ of type" + ).format(wf_with_output_error.name), + ): + wf_with_output_error(a=incorrect_input) + + +@given(st.integers()) +def test_workflow_with_output_error(correct_input): + with pytest.raises( + TypeError, + match=( + r"Encountered error while executing workflow '{}':\n" + r" Failed to convert output in position 0 of value .+, expected type \" + ).format(wf_with_output_error.name), + ): + wf_with_output_error(a=correct_input) + + +@pytest.mark.parametrize( + "workflow, position", + [ + (wf_with_multioutput_error0, 0), + (wf_with_multioutput_error1, 1), + ] +) +@given(st.integers()) +def test_workflow_with_multioutput_error(workflow, position, correct_input): + with pytest.raises( + TypeError, + match=( + r"Encountered error while executing workflow '{}':\n " + r"Failed to convert output in position {} of value .+, expected type \" + ).format(workflow.name, position) + ): + workflow(a=correct_input, b=correct_input) diff --git a/tests/flytekit/unit/core/test_type_hints.py b/tests/flytekit/unit/core/test_type_hints.py index 9ef8829c1d..412ec23cc1 100644 --- a/tests/flytekit/unit/core/test_type_hints.py +++ b/tests/flytekit/unit/core/test_type_hints.py @@ -1630,8 +1630,8 @@ def foo3(a: typing.Dict) -> typing.Dict: with pytest.raises( TypeError, match=( - "Failed to convert inputs in task 'tests.flytekit.unit.core.test_type_hints.foo':\n " - "Value 'hello' of type is not an instance of " + "Failed to convert inputs of task 'tests.flytekit.unit.core.test_type_hints.foo':\n" + " Failed argument 'a': Expected value of type but got 'hello' of type " ), ): foo(a="hello", b=10) # type: ignore @@ -1639,8 +1639,8 @@ def foo3(a: typing.Dict) -> typing.Dict: with pytest.raises( TypeError, match=( - "Failed to convert outputs of task 'tests.flytekit.unit.core.test_type_hints.foo2' at position 0:\n " - "Value 'hello' of type is not an instance of " + "Failed to convert outputs of task 'tests.flytekit.unit.core.test_type_hints.foo2' at position 0:\n" + " Expected value of type but got 'hello' of type " ), ): foo2(a=10, b="hello") @@ -1685,9 +1685,9 @@ def wf2(a: typing.Union[int, str]) -> typing.Union[int, str]: with pytest.raises( TypeError, match=re.escape( - "Error encountered while executing 'wf2':\n " - "Failed to convert inputs in task 'tests.flytekit.unit.core.test_type_hints.t2':\n " - 'Cannot convert from to typing.Union[float, dict] (using tag str)' ), ): From 2a49fd47bdaae87c090a4f419b0d0fee1e1b1484 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 15:48:11 -0400 Subject: [PATCH 06/13] add rich Signed-off-by: Niels Bantilan --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 563ce6b7d5..ab3db4c4b6 100644 --- a/setup.py +++ b/setup.py @@ -73,6 +73,7 @@ "numpy", "gitpython", "kubernetes>=12.0.1", + "rich", ], extras_require=extras_require, scripts=[ From 7850f742ab11cf5fa068a3629d340726de37c0b4 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 15:54:32 -0400 Subject: [PATCH 07/13] fix lint Signed-off-by: Niels Bantilan --- flytekit/core/promise.py | 1 - .../unit/core/test_type_conversion_errors.py | 15 ++++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/flytekit/core/promise.py b/flytekit/core/promise.py index 2b1be4e9b6..d2ac3b81a7 100644 --- a/flytekit/core/promise.py +++ b/flytekit/core/promise.py @@ -146,7 +146,6 @@ def extract_value( except TypeTransformerFailedError as exc: raise TypeTransformerFailedError(f"Failed argument '{k}': {exc}") from exc - return result diff --git a/tests/flytekit/unit/core/test_type_conversion_errors.py b/tests/flytekit/unit/core/test_type_conversion_errors.py index 71227b9f62..807bbcad22 100644 --- a/tests/flytekit/unit/core/test_type_conversion_errors.py +++ b/tests/flytekit/unit/core/test_type_conversion_errors.py @@ -4,7 +4,8 @@ from typing import Tuple import pytest -from hypothesis import given, strategies as st +from hypothesis import given +from hypothesis import strategies as st from flytekit import task, workflow @@ -16,7 +17,7 @@ def int_to_float(n: int) -> float: @task def task_incorrect_output(a: float) -> int: - return str(a) + return str(a) # type: ignore [return-value] @task @@ -55,7 +56,7 @@ def test_task_input_error(incorrect_input): match=( r"Failed to convert inputs of task '{}':\n" r" Failed argument 'a': Expected value of type \ but got .+ of type .+" - ).format(task_correct_output.name) + ).format(task_correct_output.name), ): task_correct_output(a=incorrect_input) @@ -67,7 +68,7 @@ def test_task_output_error(correct_input): match=( r"Failed to convert outputs of task '{}' at position 0:\n" r" Expected value of type \ but got .+ of type .+" - ).format(task_incorrect_output.name) + ).format(task_incorrect_output.name), ): task_incorrect_output(a=correct_input) @@ -81,7 +82,7 @@ def test_workflow_with_task_error(correct_input): r" Error encountered while executing 'wf_with_task_error':\n" r" Failed to convert outputs of task '.+' at position 0:\n" r" Expected value of type \ but got .+ of type .+" - ).format(wf_with_task_error.name) + ).format(wf_with_task_error.name), ): wf_with_task_error(a=correct_input) @@ -116,7 +117,7 @@ def test_workflow_with_output_error(correct_input): [ (wf_with_multioutput_error0, 0), (wf_with_multioutput_error1, 1), - ] + ], ) @given(st.integers()) def test_workflow_with_multioutput_error(workflow, position, correct_input): @@ -125,6 +126,6 @@ def test_workflow_with_multioutput_error(workflow, position, correct_input): match=( r"Encountered error while executing workflow '{}':\n " r"Failed to convert output in position {} of value .+, expected type \" - ).format(workflow.name, position) + ).format(workflow.name, position), ): workflow(a=correct_input, b=correct_input) From e2ace052a9636e51476e75fabcaf5ad3488b43b7 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 16:09:46 -0400 Subject: [PATCH 08/13] remove prototyping script, update loggers Signed-off-by: Niels Bantilan --- flytekit/loggers.py | 7 +++++- type_error_conversion.py | 53 ---------------------------------------- 2 files changed, 6 insertions(+), 54 deletions(-) delete mode 100644 type_error_conversion.py diff --git a/flytekit/loggers.py b/flytekit/loggers.py index 79481e9a71..45081e21b7 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -36,12 +36,17 @@ user_space_logger = child_loggers["user_space"] # create console handler +try: + console = Console(width=os.get_terminal_size().columns) +except OSError: + console = None + handler = RichHandler( rich_tracebacks=True, omit_repeated_times=False, keywords=["[flytekit]"], log_time_format="%Y-%m-%d %H:%M:%S,%f", - console=Console(width=os.get_terminal_size().columns), + console=console, ) handler.setLevel(logging.DEBUG) diff --git a/type_error_conversion.py b/type_error_conversion.py deleted file mode 100644 index 9334ec5843..0000000000 --- a/type_error_conversion.py +++ /dev/null @@ -1,53 +0,0 @@ -import random -from typing import Tuple - -from flytekit import task, workflow - - -@task -def add_rand(n: int) -> float: - return float(n + random.randint(-1000, 1000)) - - -@task -def bad_types(a: int) -> float: - return str(a) - - -@task -def good_types(a: float) -> str: - return str(a) - - -@workflow -def wf(a: int, b: int): - bad_types(a=add_rand(n=a)) - - -@workflow -def wf_bad_output(a: int, b: int) -> int: - return good_types(a=add_rand(n=a)) - - -@workflow -def wf_bad_multioutput1(a: int, b: int) -> Tuple[int, str]: - out1 = good_types(a=add_rand(n=a)) - out2 = good_types(a=add_rand(n=b)) - return out1, out2 - - -@workflow -def wf_bad_multioutput2(a: int, b: int) -> Tuple[str, int]: - out1 = good_types(a=add_rand(n=a)) - out2 = good_types(a=add_rand(n=b)) - return out1, out2 - - -if __name__ == "__main__": - # wf(a=1, b=1) - # wf(a=1, b=1.0) - # wf_bad_output(a=1, b=1) - # wf_bad_multioutput(a=1, b=2) - # wf_bad_multioutput2(a=1, b=2) - bad_types(a=1) - # bad_types(a=str(1)) From 4ec2b71b4064ca5bf058c97e2487244e74ed1578 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 16:26:45 -0400 Subject: [PATCH 09/13] update __init__.py Signed-off-by: Niels Bantilan --- flytekit/__init__.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/flytekit/__init__.py b/flytekit/__init__.py index d716b26716..f510af5825 100644 --- a/flytekit/__init__.py +++ b/flytekit/__init__.py @@ -194,6 +194,8 @@ import sys from typing import Generator +from rich import traceback + if sys.version_info < (3, 10): from importlib_metadata import entry_points else: @@ -298,9 +300,4 @@ def load_implicit_plugins(): load_implicit_plugins() # Pretty-print exception messages -try: - from rich import traceback - - traceback.install(width=None, extra_lines=0) -except ImportError: - pass +traceback.install(width=None, extra_lines=0) From 3d923e698fa34917f45ebf2c3634583d4afbfc48 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Thu, 6 Apr 2023 17:33:39 -0400 Subject: [PATCH 10/13] update logger Signed-off-by: Niels Bantilan --- flytekit/loggers.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/flytekit/loggers.py b/flytekit/loggers.py index 45081e21b7..e2c47c203c 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -1,5 +1,6 @@ import logging import os +import shutil from pythonjsonlogger import jsonlogger from rich.console import Console @@ -37,17 +38,15 @@ # create console handler try: - console = Console(width=os.get_terminal_size().columns) + handler = RichHandler( + rich_tracebacks=True, + omit_repeated_times=False, + keywords=["[flytekit]"], + log_time_format="%Y-%m-%d %H:%M:%S,%f", + console=Console(width=shutil.get_terminal_size().columns), + ) except OSError: - console = None - -handler = RichHandler( - rich_tracebacks=True, - omit_repeated_times=False, - keywords=["[flytekit]"], - log_time_format="%Y-%m-%d %H:%M:%S,%f", - console=console, -) + handler = logging.StreamHandler() handler.setLevel(logging.DEBUG) From 369ce0463acae4c12c3c3ba8593e5ed3269a027d Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Tue, 11 Apr 2023 10:41:25 -0400 Subject: [PATCH 11/13] update logger Signed-off-by: Niels Bantilan --- flytekit/loggers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flytekit/loggers.py b/flytekit/loggers.py index e2c47c203c..fdc3c75d3a 100644 --- a/flytekit/loggers.py +++ b/flytekit/loggers.py @@ -1,6 +1,5 @@ import logging import os -import shutil from pythonjsonlogger import jsonlogger from rich.console import Console @@ -43,7 +42,7 @@ omit_repeated_times=False, keywords=["[flytekit]"], log_time_format="%Y-%m-%d %H:%M:%S,%f", - console=Console(width=shutil.get_terminal_size().columns), + console=Console(width=os.get_terminal_size().columns), ) except OSError: handler = logging.StreamHandler() From 2769dedecafbda8497cec14b59aa510249d7f613 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Tue, 11 Apr 2023 13:15:57 -0400 Subject: [PATCH 12/13] fix GE and pandera tests Signed-off-by: Niels Bantilan --- flytekit/core/base_task.py | 2 +- flytekit/core/workflow.py | 5 +++-- plugins/flytekit-pandera/tests/test_plugin.py | 17 +++++++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/flytekit/core/base_task.py b/flytekit/core/base_task.py index c2ec5f0190..829a4a1332 100644 --- a/flytekit/core/base_task.py +++ b/flytekit/core/base_task.py @@ -527,7 +527,7 @@ def dispatch_execute( except Exception as exc: msg = f"Failed to convert inputs of task '{self.name}':\n {exc}" logger.error(msg) - raise TypeError(msg) from exc + raise type(exc)(msg) from exc # TODO: Logger should auto inject the current context information to indicate if the task is running within # a workflow or a subworkflow etc diff --git a/flytekit/core/workflow.py b/flytekit/core/workflow.py index da34b98229..465d74cd66 100644 --- a/flytekit/core/workflow.py +++ b/flytekit/core/workflow.py @@ -262,8 +262,9 @@ def __call__(self, *args, **kwargs) -> Union[Tuple[Promise], Promise, VoidPromis self.compile() try: return flyte_entity_call_handler(self, *args, **input_kwargs) - except TypeError as exc: - raise TypeError(f"Encountered error while executing workflow '{self.name}':\n {exc}") from exc + except Exception as exc: + exc.args = (f"Encountered error while executing workflow '{self.name}':\n {exc}", *exc.args[1:]) + raise exc def execute(self, **kwargs): raise Exception("Should not be called") diff --git a/plugins/flytekit-pandera/tests/test_plugin.py b/plugins/flytekit-pandera/tests/test_plugin.py index cc9b26c4fa..3aa9bb4d6d 100644 --- a/plugins/flytekit-pandera/tests/test_plugin.py +++ b/plugins/flytekit-pandera/tests/test_plugin.py @@ -55,7 +55,13 @@ def invalid_wf() -> pandera.typing.DataFrame[OutSchema]: def wf_with_df_input(df: pandera.typing.DataFrame[InSchema]) -> pandera.typing.DataFrame[OutSchema]: return transform2(df=transform1(df=df)) - with pytest.raises(pandera.errors.SchemaError, match="^expected series 'col2' to have type float64, got object"): + with pytest.raises( + pandera.errors.SchemaError, + match=( + "^Encountered error while executing workflow 'test_plugin.wf_with_df_input':\n" + " expected series 'col2' to have type float64, got object" + ) + ): wf_with_df_input(df=invalid_df) # raise error when executing workflow with invalid output @@ -67,7 +73,14 @@ def transform2_noop(df: pandera.typing.DataFrame[IntermediateSchema]) -> pandera def wf_invalid_output(df: pandera.typing.DataFrame[InSchema]) -> pandera.typing.DataFrame[OutSchema]: return transform2_noop(df=transform1(df=df)) - with pytest.raises(TypeError, match="^Failed to convert return value"): + with pytest.raises( + TypeError, + match=( + "^Encountered error while executing workflow 'test_plugin.wf_invalid_output':\n" + " Error encountered while executing 'wf_invalid_output':\n" + " Failed to convert outputs of task" + ), + ): wf_invalid_output(df=valid_df) From 686d1cf52dae83db61bb924662aa41bd04e632b9 Mon Sep 17 00:00:00 2001 From: Niels Bantilan Date: Tue, 11 Apr 2023 14:11:56 -0400 Subject: [PATCH 13/13] fix lint Signed-off-by: Niels Bantilan --- plugins/flytekit-pandera/tests/test_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/flytekit-pandera/tests/test_plugin.py b/plugins/flytekit-pandera/tests/test_plugin.py index 3aa9bb4d6d..7e73aac932 100644 --- a/plugins/flytekit-pandera/tests/test_plugin.py +++ b/plugins/flytekit-pandera/tests/test_plugin.py @@ -60,7 +60,7 @@ def wf_with_df_input(df: pandera.typing.DataFrame[InSchema]) -> pandera.typing.D match=( "^Encountered error while executing workflow 'test_plugin.wf_with_df_input':\n" " expected series 'col2' to have type float64, got object" - ) + ), ): wf_with_df_input(df=invalid_df)