<@U06RKAMK0UC> Do Flyte Sensors have a way to set ...
# ask-ai
a
@ancient-wolf-19325 Do Flyte Sensors have a way to set a timeout?
a
Currently, Flyte sensors do not support setting a timeout directly in their configuration. However, there is ongoing discussion and a proposed feature to add this functionality. You can follow the progress on this feature in the GitHub issue #5035. Would you like more detail on this topic? Sources: - GitHub issue #5035 👍 0 button 👎 0 button 💡 Hint: Mention @runllm in the thread for followups.
f
Ohh man we should work on this, cc @glamorous-carpet-83516 doesnt the task timeout work with this automatically?
g
it should work. did you try
Copy code
FileSensor(name="test_sensor", timeout=60)
d
I think the implementation is simple. 1. record the 1st time we poke the file (use a bool variable and datetime module) 2. everytime we poke the file, get the current time and calculate the timedelta to decide whether return true or not I can have a quick fix today
a
Sweet! Yea, I was thinking the same.
d
Ok, let me do it today
f
What is the expectation of timeout? Fail on timeout right? If so this should be automatic- flutes retry and timeout system should work
I think we are probably not passing timeout into the node
m
If we set timeout of 60 minutes, and timeout expires then workflow will fail or it will be rescheduled ?
d
it will retry if needed
it will retry if needed
Depend on your setting on flytepropeller
f
I think if you have retries it can retry else fail
m
oh ok. thanks
d
For example, I remember the default retry number is 10.
f
If we want it to still continue then we should add a new field called wait_for_time or something to that effect. Timeout implies failure
👍 1
d
It can be modified, do you need it?
f
I think @damp-lion-88352 this should set the value in core Flyte node, default should be 0,
d
Ok, will talk to you or Kevin today if there's anything unclear
thank you
a
Did you get a change to discuss this? I'm curious what you came up with
Was it something like this:
Copy code
class FileSensor(BaseSensor): # type: ignore

    def __init__(self, name: str | None = None, timeout: int = 60, **kwargs: Any) -> None:
        super().__init__(name=name, timeout=timeout, **kwargs) # type: ignore
        self.initial_poke = datetime.now()

    async def poke(self, path: str) -> bool:
        diff_in_seconds = (datetime.now() - self.initial_poke).total_seconds()
        if diff_in_seconds > self.timeout:
            raise FlyteException(f"Timed out after {diff_in_seconds}")
d
No, we are not going to do this, there's a better way to implement it in flytepropeller. Currently, I am working on other higher priority
sorry for the late reply
a
is there an issue I could follow w/ more details?
d
I can write more details about this issue, and you can implement it
probably next week
I have higher priority this week
a
ok thanks
🙏 1
d
Need 1 more day
I have lots of priority today
if finished, tomorrow will try to reply this.
🙏 1
Hi, sorry for the later reply I opened a PR HERE. https://github.com/flyteorg/flytekit/pull/2745
after remote execution is merged, are you interested in implementing local execution?
or I can do it when I have time.
a
I could look into it, but have no concept of what this would involve. I don't mind digging though
Also, left a comment on the PR
d
thank you
will ship it asap, and will go to sleep later
Copy code
from flytekit import task, workflow
from flytekit.core.task import TaskMetadata
from flytekit.sensor.file_sensor import FileSensor

sensor = FileSensor(name="test_file_sensor", metadata=TaskMetadata(timeout=5))

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


@workflow()
def wf():
    sensor(path="./hello") >> t1()
@alert-oil-1341 you can use this first
this don't need PR to be merged and you can use it
we have some changes need to also be done for backward compatibility. Thank you for your advice, I will do it in my extra ime
extra time