feat: merge attemptDeadlineSeconds and timeoutSeconds option for v2 scheduled functions

[taeold]

We are removing explicit control for users to set attemptDeadlineSeconds. This is because we feel that attemptDeadlineSeconds and timeoutSeconds for the function is deeply coupled - there isn't a valid use case we can think of that requires the two values to be different.

Per Cloud Scheduler docs:

The deadline for job attempts. If the request handler does not respond by this deadline then the request is cancelled and the attempt is marked as a DEADLINE_EXCEEDED failure. The failed attempt can be viewed in execution logs. Cloud Scheduler will retry the job according to the RetryConfig.

The default and the allowed values depend on the type of target:

For HTTP targets, the default is 3 minutes. The deadline must be in the interval [15 seconds, 30 minutes].

Effectively, we believe function timeout ~= attemptDeadline (ideally attemptDeadline should be a second or two greater than function timeout, but we leave that out as edge case for now)

[gemini-code-assist]

Summary of Changes

Hello @taeold, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request simplifies the configuration of scheduled functions by unifying the attemptDeadlineSeconds with the function's timeoutSeconds. This change eliminates the need for users to explicitly set attemptDeadlineSeconds, as the system will now automatically derive it from the function's timeout. The underlying rationale is that these two values are inherently linked and should generally be aligned, thus reducing configuration complexity and potential for discrepancies.

Highlights

• Unified Timeout Configuration: The attemptDeadlineSeconds for scheduled functions is now automatically set to the timeoutSeconds value, removing the need for separate configuration.
• Simplified User Control: Explicit user control over attemptDeadlineSeconds has been removed, streamlining the configuration process for scheduled functions.
• Manifest Update: The ScheduleTrigger definition in the manifest now includes attemptDeadlineSeconds to reflect this change.
• New Test Case: A new test has been added to verify that attemptDeadlineSeconds is correctly assigned the timeoutSeconds value.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to unify the timeoutSeconds of a scheduled function with the attemptDeadlineSeconds of the underlying Cloud Scheduler job. The changes correctly propagate the timeout value in firebase_functions.options.py, update the manifest definition, and add a new test case.

My review identifies a couple of issues with the current implementation. First, the unification of timeouts does not work correctly when default values are used, leading to different timeout values for the function and the scheduler job. Second, the provided timeout value is not validated against the allowed range for Cloud Scheduler's attemptDeadlineSeconds, which could result in deployment failures. I've provided a code suggestion to address these points.

I've also suggested an additional test case to cover the default timeout scenario and ensure the fix is verified.

[gemini-code-assist]

To ensure the unification of timeouts works correctly for the default case (when timeout_sec is not specified), it would be beneficial to add a test case that covers this scenario. This will help verify that attemptDeadlineSeconds is set to the function's default timeout (60s) instead of Cloud Scheduler's default.

    def test_on_schedule_with_default_timeout(self):
        """
        Tests that attemptDeadlineSeconds is set to the default timeoutSeconds.
        """
        decorated_func = scheduler_fn.on_schedule(
            schedule="* * * * *",
        )(Mock(__name__="example_func"))
        endpoint = decorated_func.__firebase_endpoint__
        # The function timeout will be the platform default, but the attempt deadline
        # should be explicitly set to the function's default timeout (60s).
        self.assertEqual(endpoint.timeoutSeconds, scheduler_fn._options.RESET_VALUE)
        self.assertEqual(endpoint.scheduleTrigger.get("attemptDeadlineSeconds"), 60)

[taeold]

disagree. scheduler having a default timeout of 3m. functions have default timeout of 1m. when scheudler timeout > function timeout, we don't expect things to go wrong.