feat: add time_remaining_secs property to MIGRATING event data#940
Conversation
|
|
||
| # The remaining time in seconds before the migration is forced and the process is killed | ||
| # Optional because it's not present when the event handler is called manually | ||
| time_remaining_secs: Annotated[float | None, Field(alias='timeRemainingSecs')] |
There was a problem hiding this comment.
Could we use datetime/time types rather than float + secs suffix identifier?
There was a problem hiding this comment.
for durations, we usually do timedelta
There was a problem hiding this comment.
That's very Python-specific, it would be hard to keep parity between JS and Python there.
Plus this way (with seconds) it's easier to use with AutoscaledPool:
Actor.on('migrating', ({ timeRemainingSecs }) => {
await crawler.autoscaledPool.pause(0.9 * timeRemainingSecs);
await persistStateSomehow();
await Actor.reboot();
});There was a problem hiding this comment.
But if you want it as timedelta in Python, I can add it, it shouldn't be hard. Only the property names will be slightly different between JS and Python, but I guess it's fine.
There was a problem hiding this comment.
Yes please. We use timedelta for all time-related fields. For Pydantic models, we have a custom type called timedelta_ms, which includes a custom validator and serializer. You can see it in use e.g. here.
And the property will only differ in Python’s internal code, the env var name can remain unchanged.
Edit: ah, of course, sorry...
There was a problem hiding this comment.
It's not an env var, it's data in an Actor event, and it's not internal, since it's something that users will be working with.
I'll rename the field exposed to users to time_remaining, which will be a timedelta, and internally it will be parsed from the timeRemainingSecs property of the received event data.
The Apify platform now sends a
timeRemainingSecsproperty in theMIGRATINGevent data, this adds it to the event data model.