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

Improve error message for missing return #2551

Merged
merged 32 commits into from
Jul 15, 2024
Merged

Improve error message for missing return #2551

merged 32 commits into from
Jul 15, 2024

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Jul 3, 2024

Tracking issue

NA

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

import os

from click.testing import CliRunner

from flytekit import task, workflow, ImageSpec
from flytekit.clis.sdk_in_container import pyflyte

default = ["pandas", "mypy"]

image = ImageSpec(registry="pingsutw", packages=[*default])


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


@task(container_image=os.getenv("IMAGE"))
def t2(a: int) -> int:
    return a + 3


@workflow()
def wf(a: int = 3) -> int:
    t1()
    t2(a=a)


if __name__ == '__main__':
    runner = CliRunner()
    result = runner.invoke(
        pyflyte.main,
        [
            "-vv",
            "register",
            "/Users/kevin/git/flytekit/flyte-example/improve_image_spec.py",
        ]
    )
    print(result.stdout)

Setup process

Screenshots

Before:

Screenshot 2024-07-03 at 1 37 55 AM

After:
Screenshot 2024-07-03 at 1 37 46 AM

Check all the applicable boxes

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

Related PRs

NA

Docs link

NA

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]>
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]>
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]>
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 pingsutw marked this pull request as ready for review July 12, 2024 22:25
Signed-off-by: Kevin Su <[email protected]>
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 41.66667% with 21 lines in your changes missing coverage. Please review.

Project coverage is 41.64%. Comparing base (993ff59) to head (13d7793).
Report is 4 commits behind head on master.

Files Patch % Lines
flytekit/core/utils.py 30.00% 7 Missing ⚠️
flytekit/exceptions/utils.py 0.00% 6 Missing ⚠️
flytekit/core/interface.py 42.85% 4 Missing ⚠️
flytekit/exceptions/user.py 69.23% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2551      +/-   ##
==========================================
- Coverage   41.67%   41.64%   -0.03%     
==========================================
  Files         184      184              
  Lines       18784    18802      +18     
  Branches     3926     3708     -218     
==========================================
+ Hits         7828     7830       +2     
- Misses      10824    10863      +39     
+ Partials      132      109      -23     

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

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]>
Signed-off-by: Kevin Su <[email protected]>
Future-Outlier
Future-Outlier previously approved these changes Jul 15, 2024
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw merged commit 7a6fe0a into master Jul 15, 2024
46 checks passed
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
mao3267 pushed a commit to mao3267/flytekit that referenced this pull request Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants