fix: rename task timeout kind and also apply it to scheduler#1934
fix: rename task timeout kind and also apply it to scheduler#1934IzaakGough wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increases the maximum timeout limit for scheduled functions (v2) from 540 seconds to 1800 seconds. To support this, the timeout category "task" has been renamed to "scheduledOrTask", and the constant "MAX_TASK_TIMEOUT_SECONDS" has been updated to "MAX_SCHEDULE_OR_TASK_TIMEOUT_SECONDS". Validation logic, tests, and error messages have been updated accordingly. I have no additional feedback to provide as there are no review comments.
cabljac
left a comment
cabljac
left a comment
There was a problem hiding this comment.
Approving. Two inline comments: the doc gap is worth fixing in this PR, the naming one is take it or leave it.
| /** | ||
| * Maximum timeout in seconds for event-handling functions (e.g. Firestore, | ||
| * Realtime Database, Pub/Sub, Storage, Eventarc, alerts, scheduler). | ||
| * Realtime Database, Pub/Sub, Storage, Eventarc, alerts). |
There was a problem hiding this comment.
Nice catch applying the kind to the scheduler too, and the boundary tests are exactly right.
One doc gap: the GlobalOptions.timeoutSeconds @remarks (around line 202) still only lists task queue functions at 1,800s, so the generated reference docs won't tell anyone that scheduled functions now share that cap. Please update that sentence to something like "Task queue and scheduled functions have a maximum timeout of 1,800s (30 minutes)." The same stale copy is on TaskQueueOptions.timeoutSeconds in tasks.ts:87, worth fixing both in this PR since it's the same sentence.
(That sentence is copy-pasted across most provider option types too, e.g. pubsub.ts, https.ts, storage.ts. Fine to leave those for a follow-up, but flagging so we don't lose track.)
3 participants
Fixes #1933
This PR renames the "task" timeout kind to "scheduledOrTask" and ensures that both task and scheduler functions use this. The documentation states that task and scheduler functions should have the same timeout settings, but scheduler functions were falling back to a generic "event" timeout kind.
Testing
relnote: none