flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard
verified publisher icon flyteorg

[wip] FileSensor Timeout (Remote Execution Only)

Open Future-Outlier opened this issue 1 year ago • 2 comments

Tracking issue

https://github.com/flyteorg/flyte/issues/5035

Why are the changes needed?

timout is important for agent tasks.

What changes were proposed in this pull request?

How was this patch tested?

from flytekit import task, workflow
from flytekit.sensor.file_sensor import FileSensor

sensor = FileSensor(name="test_file_sensor", timeout=10)


# %%
@task()
def t1():
    print("SUCCEEDED")


@workflow()
def wf():
    sensor(path=".") >> t1()


if __name__ == "__main__":
    from flytekit.clis.sdk_in_container import pyflyte
    from click.testing import CliRunner

    runner = CliRunner()
    path = "/Users/future-outlier/code/dev/flytekit/build/PR/agent/sensor_task.py"
    result = runner.invoke(pyflyte.main, ["run", path, "wf",])

    print("Local Execution: ", result.output)
    #
    result = runner.invoke(pyflyte.main, ["run", "--remote", path, "wf"])
    print("Remote Execution: ", result.output)

Setup process

Screenshots

image image image

Check all the applicable boxes

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

Related PRs

Docs link

Future-Outlier avatar Sep 11 '24 15:09 Future-Outlier

Hi, @pingsutw Do you think it's a good idea to support timeout by Resource Wrapper in flyteplugins/agent? https://github.com/flyteorg/flyte/blob/master/flyteplugins/go/tasks/plugins/webapi/agent/plugin.go#L40-L48

Or should we make this feature more generic?

Update: it's done already!

Future-Outlier avatar Sep 11 '24 15:09 Future-Outlier

Codecov Report

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

Project coverage is 79.34%. Comparing base (5012ded) to head (e89094a). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
flytekit/extend/backend/base_agent.py 20.00% 4 Missing :warning:
flytekit/sensor/base_sensor.py 66.66% 1 Missing and 1 partial :warning:
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2745      +/-   ##
==========================================
+ Coverage   78.12%   79.34%   +1.21%     
==========================================
  Files         307      213      -94     
  Lines       26968    22315    -4653     
  Branches     2911     2918       +7     
==========================================
- Hits        21070    17705    -3365     
+ Misses       5040     3803    -1237     
+ Partials      858      807      -51

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: 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.

codecov[bot] avatar Sep 11 '24 15:09 codecov[bot]

any chance we could revive this?

davidmirror-ops avatar Feb 04 '25 16:02 davidmirror-ops

Yes please

kumare3 avatar Feb 08 '25 18:02 kumare3

Code Review Agent Run #b590ee

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: 0b1d40d..c964bf0
    • flytekit/core/base_task.py
    • flytekit/extend/backend/base_agent.py
    • flytekit/sensor/base_sensor.py
    • flytekit/sensor/file_sensor.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

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

flyte-bot avatar Mar 12 '25 03:03 flyte-bot

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - Agent Timeout Warning Enhancement

- base_agent.py - Added logger import and inserted warnings in SyncAgentExecutorMixin and AsyncAgentExecutorMixin to log when timeout is specified for local execution.
New Feature - Sensor Timeout Support

- base_sensor.py - Integrated a timeout parameter with TaskMetadata and added conflict checks to prevent simultaneous use of timeout and metadata.

- file_sensor.py - Updated the constructor to accept a timeout parameter and pass it to the parent, enhancing sensor configuration.
Testing - Sensor Timeout Testing

- test_file_sensor.py - Introduced comprehensive tests to verify timeout initialization, conversion from int to timedelta, and proper error handling in sensor tasks.

flyte-bot avatar Mar 12 '25 03:03 flyte-bot

it work again now! image

Future-Outlier avatar Mar 13 '25 01:03 Future-Outlier

Code Review Agent Run #1e462d

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: c964bf0..8c08059
    • flytekit/core/base_task.py
    • flytekit/sensor/base_sensor.py
    • tests/flytekit/unit/sensor/test_file_sensor.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

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

flyte-bot avatar Mar 13 '25 02:03 flyte-bot

Code Review Agent Run #c19f04

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 8c08059..e89094a
    • flytekit/sensor/base_sensor.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

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

flyte-bot avatar Mar 13 '25 05:03 flyte-bot

Code Review Agent Run #fc34c3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: e89094a..d2c5f14
    • tests/flytekit/unit/sensor/test_file_sensor.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

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

flyte-bot avatar Mar 14 '25 02:03 flyte-bot

Minor nit otherwise LGTM

yes done, thank you!

Future-Outlier avatar Mar 14 '25 02:03 Future-Outlier