Improve error message for MismatchingTypes

[pingsutw]

Tracking issue

NA

Why are the changes needed?

The compilation error message is not clear to users. The error message only shows the node ID, so users don't know which task/workflow has problems.

What changes were proposed in this pull request?

Add task/workflow name to the error message.

How was this patch tested?

pyflyte run --remote flyte-example/improve_error/mismatching_type.py wf3

Setup process

import typing

from flytekit import task, workflow, ImageSpec

image = ImageSpec(registry="pingsutw", packages=["pandas", "mypy"])


@task(container_image=image)
def t1() -> int:
    return 3 + 2


@task(container_image=image)
def t2(a: str) -> int:
    print(a)
    return 3


@task()
def calculate_circle_circumference(radius: float) -> typing.Optional[float]:
    return 2 * 3.14 * radius  # Task to calculate the circumference of a circle


@task()
def calculate_circle_area(radius: float) -> typing.Optional[float]:
    return 3.14 * radius * radius  # Task to calculate the area of a circle


@task
def echo(a: float) -> typing.Optional[float]:
    return a


@workflow()
def subwf1() -> int:
    return t1()


@workflow()
def subwf2(a: str) -> int:
    return t2(a=a)


@workflow()
def wf():
    subwf2(a=t1())  # (promise) task -> workflow


@workflow()
def wf1():
    t2(a=t1())  # (promise) task -> task


@workflow()
def wf2():
    subwf2(a=subwf1())  # (promise) workflow -> workflow


@workflow()
def wf3():
    t2(a=subwf1())  # (promise) workflow -> task

Screenshots

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

NA

Docs link

NA

[codecov]

Codecov Report

Attention: Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.

Project coverage is 36.19%. Comparing base (37b4e13) to head (c71c20b).
Report is 174 commits behind head on master.

Files with missing lines	Patch %	Lines
...tepropeller/pkg/compiler/errors/compiler_errors.go	42.85%	4 Missing ⚠️
flyteadmin/pkg/workflowengine/impl/compiler.go	0.00%	1 Missing ⚠️
flytepropeller/pkg/compiler/validators/bindings.go	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5639      +/-   ##
==========================================
+ Coverage   36.17%   36.19%   +0.01%     
==========================================
  Files        1302     1302              
  Lines      109484   109627     +143     
==========================================
+ Hits        39609    39677      +68     
- Misses      65737    65803      +66     
- Partials     4138     4147       +9     

Flag	Coverage Δ
unittests-datacatalog	51.37% <ø> (ø)
unittests-flyteadmin	55.33% <50.00%> (+<0.01%)	⬆️
unittests-flytecopilot	12.17% <ø> (ø)
unittests-flytectl	62.18% <ø> (-0.14%)	⬇️
unittests-flyteidl	7.12% <ø> (+0.04%)	⬆️
unittests-flyteplugins	53.34% <ø> (+0.03%)	⬆️
unittests-flytepropeller	41.76% <76.19%> (+0.02%)	⬆️
unittests-flytestdlib	55.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[eapolinario]

This is easily one of the most useful PRs coming out of this effort to improve error messages. Thank you for working on it!

[eapolinario]

is this right?

[pingsutw]

yup, outputVar is a promise.

flyte/flytepropeller/pkg/compiler/validators/bindings.go

Line 126 in 827d55b

outputVar = fmt.Sprintf("%s.%s", upNode.GetMetadata().Name, val.Promise.Var)

[eapolinario]

Should we have 2 different functions instead? One to handle the previous case and another one to handle the new case where toVar is set.

[pingsutw]

that's better. updated it, thanks~

[Future-Outlier]

The output variable ... the input variable

The upstream variable ... the downstream variable
Do you think it will be better?

[Future-Outlier]

Wait, I think the version now is better.

        Details: failed to compile workflow for [resource_type:WORKFLOW  project:"flytesnacks"  domain:"development"  name:"example.wf3"  version:"ArJxfyEQms9wkDQCL-2Tpw"] with err: Collected Errors: 1
        Error 0: Code: MismatchingTypes, Node Id: n1, Description: The output variable 'subwf1.o0' has type [simple:INTEGER], but it's assigned to the input variable 't2.a' which has type type [simple:STRING].

[Future-Outlier]

I'm a little bit tired today, let me review this PR tomorrow morning when I'm energetic.

[Future-Outlier]

LGTM, this is super helpful.