Add configurable timeout to requests API calls in OpenFaasHook

[Ironankit525]

All HTTP requests in OpenFaasHook (get, post, put) now include a configurable timeout parameter (default 60s) read from the connection extra dict.

Justification:

Without a timeout, an unresponsive remote OpenFaaS server could cause Airflow execution tasks to hang indefinitely, tying up worker slots. Using the connection's extra_dejson block allows users to configure this timeout based on their expected function execution time, defaulting to 60 seconds.

This was originally submitted alongside #62990 but split out per reviewer request.

[SameerMesiah97]

I have left a comment.

[SameerMesiah97]

Not against adding timeouts here but there are 2 issues with doing it this way:

1. The timeout setting is being fetched multiple times in the hook when it's a constant.
2. You are making timeout configurable via the connection extras, which is not idiomatic for provider hooks. Also, it does not look like you are exposing the field via get_ui_field_behavior either, so it would become a hidden setting.

I think it would be better to set a top-level variable like this:

REQUEST_TIMEOUT = 60

You can put it near OK_STATUS_CODE. You can then pass this variable as an argument for the timeout parameter in this hook.

I do not see a need to expose this setting to users at the moment.

[Ironankit525]

Thanks for the feedback and for pointing that out, @SameerMesiah97. I completely agree - fetching it multiple times and relying on hidden UI variables was definitely not ideal.

I have updated the PR to use a top-level REQUEST_TIMEOUT = 60 constant near OK_STATUS_CODE and passed it directly to all the requests calls within the hook as requested.

Please let me know if there's anything else!

[SameerMesiah97]

Thanks for the feedback and for pointing that out, @SameerMesiah97. I completely agree - fetching it multiple times and relying on hidden UI variables was definitely not ideal.

I have updated the PR to use a top-level REQUEST_TIMEOUT = 60 constant near OK_STATUS_CODE and passed it directly to all the requests calls within the hook as requested.

Please let me know if there's anything else!

Let’s wait for CI to run. In the meantime, I would check if any of the unit tests assert the request calls with params. And modify them if necessary.

[dabla]

I would still try to get the timeout from the connection extra if defined, that way you can define a global timeout value on the connection level, but the timeout parameter should be an instance field of the hook and should thus also be configurable through the constructor of the hook which by default will be None.

    def __init__(self, function_name=None, conn_id: str = "open_faas_default", timeout: int | None = None, *args, **kwargs) -> None:
        super().__init__(*args, **kwargs)
        self.function_name = function_name
        self.conn_id = conn_id
        self.timeout = timeout

Next to that, in the get_conn method, where the connection is being fetched, I would check if timeout isn't defined in OpenFaasHook, and if it's not defined, then I would check in extra of connection or use the constant REQUEST_TIMEOUT defined at the top level.

    def get_conn(self):
        conn = self.get_connection(self.conn_id)
        if self.timeout is None:
            self.timeout = conn.extra_dejson.get("timeout", REQUEST_TIMEOUT)
        return conn

[potiuk]

good points

[potiuk]

@Ironankit525 This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

• ❌ Description doesn't match code: The PR description prominently claims the timeout is 'configurable' and 'read from the connection extra dict', and explicitly states 'Using the connection's extra_dejson block allows users to configure this timeout'. However, the actual implementation adds a single hardcoded module-level constant REQUEST_TIMEOUT = 60 that is never read from any connection extra field. The configurability described in the justification is entirely absent from the code — users have no way to change the timeout per-connection as advertised.
• ⚠️ Missing regression tests: No test changes are included. The existing OpenFaaS provider tests mock requests.get/post/put calls and should be updated to assert that timeout=REQUEST_TIMEOUT (or whatever the final value is) is passed in each call, covering all four methods: deploy_function, invoke_async_function, invoke_function, update_function, and does_function_exist.

Note: Your branch is 879 commits behind main. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.

What to do next:

• The comment informs you what you need to do.
• Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
• There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
• Maintainers will then proceed with a normal review.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

---

Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a timeout to OpenFaaS HTTP calls from OpenFaasHook to prevent indefinitely hanging Airflow tasks when the OpenFaaS gateway is unresponsive.

Changes:

• Introduces a module-level request timeout constant and applies it to requests.get/post/put calls within OpenFaasHook.

[Copilot]

The PR description says the timeout is configurable via the connection extra dict (defaulting to 60s), but the implementation uses a hard-coded module constant (REQUEST_TIMEOUT = 60) and does not read from extra_dejson. To match the described behavior, derive the timeout from the connection extras (e.g., timeout key) with a default fallback, and use that value for all requests. Consider renaming the constant to something like DEFAULT_REQUEST_TIMEOUT to reflect its role as a fallback rather than the enforced value.

[Copilot]

The PR description says the timeout is configurable via the connection extra dict (defaulting to 60s), but the implementation uses a hard-coded module constant (REQUEST_TIMEOUT = 60) and does not read from extra_dejson. To match the described behavior, derive the timeout from the connection extras (e.g., timeout key) with a default fallback, and use that value for all requests. Consider renaming the constant to something like DEFAULT_REQUEST_TIMEOUT to reflect its role as a fallback rather than the enforced value.

[Copilot]

These calls pass body as the second positional argument, which requests treats as data, not JSON. If the OpenFaaS endpoints expect JSON payloads (typical for the gateway API), use an explicit keyword (json=body) or at least data=... to avoid ambiguity and accidental content-type/encoding issues.

[Copilot]

These calls pass body as the second positional argument, which requests treats as data, not JSON. If the OpenFaaS endpoints expect JSON payloads (typical for the gateway API), use an explicit keyword (json=body) or at least data=... to avoid ambiguity and accidental content-type/encoding issues.

[Copilot]

These calls pass body as the second positional argument, which requests treats as data, not JSON. If the OpenFaaS endpoints expect JSON payloads (typical for the gateway API), use an explicit keyword (json=body) or at least data=... to avoid ambiguity and accidental content-type/encoding issues.

[Copilot]

These calls pass body as the second positional argument, which requests treats as data, not JSON. If the OpenFaaS endpoints expect JSON payloads (typical for the gateway API), use an explicit keyword (json=body) or at least data=... to avoid ambiguity and accidental content-type/encoding issues.