-
Notifications
You must be signed in to change notification settings - Fork 17
feat: Add RemainingTime option for timeout argument of Actor.call and Actor.start
#473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/apify/_actor.py
Outdated
| if self.is_at_home() and self.configuration.timeout_at: | ||
| return self.configuration.timeout_at - datetime.now(tz=timezone.utc) | ||
|
|
||
| self.log.warning('Using `RemainingTime` argument for timeout outside of the Apify platform. Returning `None`') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be printed out if self.is_at_home() is True, but self.configuration.timeout_at is None? I assume that this is unlikely, but still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expanded the warning
| async with Actor: | ||
| actor_input = (await Actor.get_input()) or {} | ||
| if actor_input.get('called_from_another_actor', False) is True: | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the Actor finishes immediately and you just check that the timeout was configured correctly? I agree that it's fine to trust the platform. Could you add a comment here that explains that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more comments
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
30ad913 to
31da56f
Compare
31da56f to
05eed5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Added convenient way to start another Actor with reasonable timeout to prevent lingering actor runs after main actor shuts down.
Added test.
RemainingTimevalue of the timeout argument is not a default one and has to be explicitly passed by the user. Using this value will calculate remaining time of this actor run and pass it as a timeout to the actor that is being started.Issues
RemainingTimeoption forActor.startandActor.call#472