flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.17k stars 550 forks source link

deep copying arraynode tasktemplate interface #5479

Closed hamersaw closed 2 weeks ago

hamersaw commented 2 weeks ago

Tracking issue

NA

Why are the changes needed?

ArrayNode strips the collection from the output type because it currently relies on registering a separate task with a wrapped collection to satisfy node output validators. The bug is that this stripped did no deep copy, so if multiple nodes stripped the outer collection, we would end up stripping mutliple layers if they existed.

What changes were proposed in this pull request?

In this PR we deep copy the interface of the TaskTemplate to ensure that multiple subNode executions do not strip mutliple List from the output variable type.

How was this patch tested?

The following workflow is a minimal repro:

@task(cache=True, cache_version="1.0")
def bAr(a: int) -> List[int]:
    return [a + 1, a + 2]

@task(cache=True, cache_version="1.0")
def bAr2(a: int) -> List[int]:
    return [a + 1, a + 2]

@workflow
def my_wf_2(a: int) -> List[List[int]]:
    x = bAr(a=a)
    return map_task(bAr2)(a=x)

Setup process

Screenshots

Check all the applicable boxes

Related PRs

NA

Docs link

NA

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 60.99%. Comparing base (7d788cb) to head (ddd7c8c).

Files Patch % Lines
...g/controller/nodes/array/node_execution_context.go 0.00% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #5479 +/- ## ========================================== + Coverage 60.15% 60.99% +0.83% ========================================== Files 646 793 +147 Lines 45819 51350 +5531 ========================================== + Hits 27562 31319 +3757 - Misses 15653 17145 +1492 - Partials 2604 2886 +282 ``` | [Flag](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | Coverage Δ | | |---|---|---| | [unittests-datacatalog](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `69.31% <ø> (ø)` | | | [unittests-flyteadmin](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `58.70% <ø> (ø)` | | | [unittests-flytecopilot](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `17.79% <ø> (ø)` | | | [unittests-flytectl](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `67.97% <ø> (?)` | | | [unittests-flyteidl](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `79.04% <ø> (ø)` | | | [unittests-flyteplugins](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `61.84% <ø> (ø)` | | | [unittests-flytepropeller](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `57.30% <0.00%> (-0.02%)` | :arrow_down: | | [unittests-flytestdlib](https://app.codecov.io/gh/flyteorg/flyte/pull/5479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `65.82% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.