Better error messages for Type Errors #655
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
pingsutw
force-pushed
the
arg-errors
branch
from
b188e89 to
d842541
Compare
Signed-off-by: Kevin Su <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #655 +/- ##
==========================================
+ Coverage 85.57% 85.61% +0.03%
==========================================
Files 355 355
Lines 29341 29549 +208
Branches 2379 2405 +26
==========================================
+ Hits 25110 25298 +188
- Misses 3594 3610 +16
- Partials 637 641 +4
Continue to review full report at Codecov.
|
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
pingsutw
force-pushed
the
arg-errors
branch
from
85394fb to
fd3e1d8
Compare
pingsutw
changed the title
| logger.error(f"failed to convert return value for var {k} with error {type(e)}: {e}") | ||
| raise 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}") from e |
There was a problem hiding this comment.
Have we seen how this looks on console? i remember the last time I tried the from e syntax, a lot of the stack trace was missing and it became a lot harder to debug.
There was a problem hiding this comment.
Traceback (most recent call last):
File "/Users/kevin/git/flytekit/flytekit/core/base_task.py", line 508, in dispatch_execute
literals[k] = TypeEngine.to_literal(exec_ctx, v, py_type, literal_type)
File "/Users/kevin/git/flytekit/flytekit/core/type_engine.py", line 390, in to_literal
transformer.assert_type(python_type, python_val)
File "/Users/kevin/git/flytekit/flytekit/core/type_engine.py", line 65, in assert_type
raise TypeError(f"Type of Val '{v}' is not an instance of {t}")
TypeError: Type of Val 'Welcome, kevin!' is not an instance of <class 'int'>
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "test.py", line 28, in <module>
print("Final result", welcome(name="kevin"))
File "/Users/kevin/git/flytekit/flytekit/core/workflow.py", line 236, in __call__
return flyte_entity_call_handler(self, *args, **input_kwargs)
File "/Users/kevin/git/flytekit/flytekit/core/promise.py", line 876, in flyte_entity_call_handler
result = cast(LocallyExecutable, entity).local_execute(child_ctx, **kwargs)
File "/Users/kevin/git/flytekit/flytekit/core/workflow.py", line 251, in local_execute
function_outputs = self.execute(**kwargs)
File "/Users/kevin/git/flytekit/flytekit/core/workflow.py", line 686, in execute
return exception_scopes.user_entry_point(self._workflow_function)(**kwargs)
File "/Users/kevin/git/flytekit/flytekit/common/exceptions/scopes.py", line 198, in user_entry_point
return wrapped(*args, **kwargs)
File "test.py", line 22, in welcome
greeting = greet(name=name)
File "/Users/kevin/git/flytekit/flytekit/core/base_task.py", line 274, in __call__
return flyte_entity_call_handler(self, *args, **kwargs)
File "/Users/kevin/git/flytekit/flytekit/core/promise.py", line 869, in flyte_entity_call_handler
return cast(LocallyExecutable, entity).local_execute(ctx, **kwargs)
File "/Users/kevin/git/flytekit/flytekit/core/base_task.py", line 255, in local_execute
outputs_literal_map = self.dispatch_execute(ctx, input_literal_map)
File "/Users/kevin/git/flytekit/flytekit/core/base_task.py", line 511, in dispatch_execute
raise TypeError(f"Failed to convert return value for var {k} for function {self.name}") from e
TypeError: Failed to convert return value for var o0 for function __main__.greet
There was a problem hiding this comment.
Here is an example.
https://gist.github.com/pingsutw/0b45f5413dcc11fb8c6a7c6b640dd5ad
pingsutw
previously approved these changes
Sep 23, 2021
eapolinario
reviewed
Sep 23, 2021
| def foo2(a: int, b: str) -> typing.Tuple[int, str]: | ||
| return "hello", 10 | ||
|
|
||
| with pytest.raises(TypeError): |
There was a problem hiding this comment.
can you also use the match parameter for this test? Just to confirm that the message is what we expect.
| @@ -1338,3 +1340,16 @@ def t2() -> dict: | |||
| expected_struct = Struct() | |||
| expected_struct.update({"k1": "v1", "k2": 3, "4": {"one": [1, "two", [3]]}}) | |||
| assert output_lm.literals["o0"].scalar.generic == expected_struct | |||
|
|
|||
|
|
|||
| def test_error_messages(): | |||
There was a problem hiding this comment.
can we have a test where the mismatch happens in a proto? We had an issue in the OSS slack where a workflow input of type Dictionary but it was being passed a list of dictionaries (and as you can see from the slack message, the error is not intuitive).
Signed-off-by: Kevin Su <[email protected]>
eapolinario
approved these changes
Sep 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
This PR intends to improve the error messages in case of type errors and also introduces stricter type checking for primitive types.
Type
Are all requirements met?
Complete description
How did you fix the bug, make the feature etc. Link to any design docs etc
Tracking Issue
fixes flyteorg/flyte#1439
fixes flyteorg/flyte#1423
Follow-up issue
NA