check execution phase before access outputs

[troychiu]

Why are the changes needed?

Currently if we sync an execution which contains a launch plan, the sync would fail if the execution failed. This is because we didn't check the execution phase before access the outputs.

What changes were proposed in this pull request?

Check if the execution is successful before access the output.

How was this patch tested?

Tested with

from flytekit import workflow, task, LaunchPlan

@task()
def print_head(a: int) -> int:
    print(1 / 1)
    return a + 1


@workflow()
def easy_execution(b: int) -> int:
    return print_head(a=b)


lp = LaunchPlan.create(name="lp-easy-execution", workflow=easy_execution)


@workflow()
def master(a: int) -> int:
    return lp(b=a)

r = FlyteRemote()
a = r.sync(exec, sync_nodes=True)

Summary by Bito

This PR fixes a critical bug where output access could fail for unsuccessful executions. It adds a new property to verify execution phase and updates remote logic to check execution success before accessing outputs, preventing synchronization failures from premature output access.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

[flyte-bot]

Code Review Agent Run #2137fe

Actionable Suggestions - 1

• flytekit/remote/executions.py - 1

  • Missing property decorator in implementation · Line 153-153

Review Details

• Files reviewed - 2 · Commit Range: c9df6d9..c9df6d9

  • flytekit/remote/executions.py
  • flytekit/remote/remote.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

AI Code Review powered by

[codecov]

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.66%. Comparing base (7915afa) to head (c9df6d9).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
flytekit/remote/remote.py	0.00%	2 Missing ⚠️
flytekit/remote/executions.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3179      +/-   ##
==========================================
- Coverage   78.71%   77.66%   -1.05%     
==========================================
  Files         293      213      -80     
  Lines       25790    22247    -3543     
  Branches     2897     2901       +4     
==========================================
- Hits        20300    17278    -3022     
+ Misses       4681     4119     -562     
- Partials      809      850      +41     

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

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flyte-bot]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - Preventing Erroneous Output Access	- executions.py - Introduced the 'is_successful' property to validate execution success by checking if the workflow phase is SUCCEEDED. - remote.py - Modified the logic to conditionally assign outputs only when the execution is successful, preventing premature output access.

[flyte-bot]

Missing property decorator in implementation

Consider adding a property decorator to the is_successful method to maintain consistency with other properties in the class. The method is currently defined with a property decorator but is missing the @property decorator in the implementation.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	def is_successful(self) -> bool:
	@property
	def is_successful(self) -> bool:

Code Review Run #2137fe

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them