[wip] FileSensor Timeout (Remote Execution Only)

[Future-Outlier]

Tracking issue

flyteorg/flyte#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

Check all the applicable boxes

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

Related PRs

Docs link

[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!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.03%. Comparing base (ae9c6f8) to head (6206175).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2745      +/-   ##
==========================================
+ Coverage   90.87%   95.03%   +4.16%     
==========================================
  Files          74       42      -32     
  Lines        3331     2095    -1236     
==========================================
- Hits         3027     1991    -1036     
+ Misses        304      104     -200     

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

[blaketastic2]

Would it make more sense to expose metadata to the PythonTask and have BaseSensor initialize the metadata w/ TaskMetadata(timeout=timeout)?

[Future-Outlier]

yes it will make more sense.

[thomasjpfan]

I do not like how timeout can be defined in two places. If we really want a easy way to set timeout, I would do this logic FileSensor and not complicate the Task API.