Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Non-descriptive error messages #1423

Closed
vsbus opened this issue Sep 3, 2021 · 5 comments · Fixed by flyteorg/flytekit#655
Closed

[BUG] Non-descriptive error messages #1423

vsbus opened this issue Sep 3, 2021 · 5 comments · Fixed by flyteorg/flytekit#655
Assignees
Labels
bug Something isn't working flytekit FlyteKit Python related issue

Comments

@vsbus
Copy link
Contributor

vsbus commented Sep 3, 2021

Describe the bug
I had been spending quite a bit of time debugging issues with types in the last weeks. It would be super-super helpful if error messages include names of the parameters that are causing issues. Often we try to guess the bad parameter but suspect a wrong thing and spending a lot of time trying to fix a wrong path. If error messages point to the concerning parameter / function / wf then in most cases application users will be able to debug and fix not escalating to infra team.

e.g.

ValueError: No transformers could reverse Flyte literal type simple: STRUCT

was an issue with parameter of type dict

e.g.2

AttributeError: 'NoneType' object has no attribute 'is_remote'

was an issue with FlyteContext and not any of parameters (but we spend a few days assuming a user messed up with one of FlyteFile args)

Expected behavior
error log should contain more precise pointers, like

  • should say dict instead of STRUCT b/c dict is actually used in the user code
  • ideally should provide path to the problematic parameter, e.g. workflow_name->task_name->input_name

Slack discussion: https://flyte-org.slack.com/archives/CREL4QVAQ/p1630645366078600

[Optional] Additional context
To Reproduce
Steps to reproduce the behavior:
1.
2.

Screenshots
If applicable, add screenshots to help explain your problem.

@vsbus vsbus added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Sep 3, 2021
@kumare3 kumare3 added bugSquash-Cascade flytekit FlyteKit Python related issue and removed untriaged This issues has not yet been looked at by the Maintainers labels Sep 3, 2021
@kumare3
Copy link
Contributor

kumare3 commented Sep 4, 2021

@vsbus we can definitely highlight the variable name in the error message and in most cases improve the message. But there are some cases in which we cannot do so, until we implement #1363. As Currently flytekit is more expressive in types than Flyte itself and this causes some type-erasure. But as you can see, we will work on 1363 and that should improve the error messaging - even in case of directly using FlyteRemote

@georgesnelling
Copy link
Contributor

@ketan would it make sense to separate the two fixes, and at least plumb through the erring parameter name now before we implement the entire #1363 enchilada? @vsbus would that have saved you time?

@kumare3
Copy link
Contributor

kumare3 commented Sep 5, 2021

@georgesnelling yup, that's the plan

@kumare3 kumare3 self-assigned this Sep 6, 2021
@kumare3
Copy link
Contributor

kumare3 commented Sep 6, 2021

I have a PR in progress

@vsbus
Copy link
Contributor Author

vsbus commented Sep 8, 2021

Splitting fixes makes sense. Incremental improvements totally make sense and good for our use case.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flytekit FlyteKit Python related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants