Copilot commented on code in PR #62731:
URL: https://github.com/apache/airflow/pull/62731#discussion_r3066472184
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -71,7 +72,7 @@ def deploy_function(self, overwrite_function_if_exist: bool,
body: dict[str, Any
else:
url = self.get_conn().host + self.DEPLOY_FUNCTION
self.log.info("Deploying function %s", url)
- response = requests.post(url, body)
+ response = requests.post(url, body, timeout=REQUEST_TIMEOUT)
Review Comment:
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.
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -24,6 +24,7 @@
from airflow.providers.common.compat.sdk import AirflowException, BaseHook
OK_STATUS_CODE = 202
+REQUEST_TIMEOUT = 60
Review Comment:
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.
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -93,7 +94,7 @@ def invoke_function(self, body: dict[str, Any]) -> None:
"""Invoke function synchronously. This will block until function
completes and returns."""
url = self.get_conn().host + self.INVOKE_FUNCTION + self.function_name
self.log.info("Invoking function synchronously %s", url)
- response = requests.post(url, body)
+ response = requests.post(url, body, timeout=REQUEST_TIMEOUT)
Review Comment:
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.
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -106,7 +107,7 @@ def update_function(self, body: dict[str, Any]) -> None:
"""Update OpenFaaS function."""
url = self.get_conn().host + self.UPDATE_FUNCTION
self.log.info("Updating function %s", url)
- response = requests.put(url, body)
+ response = requests.put(url, body, timeout=REQUEST_TIMEOUT)
Review Comment:
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.
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -82,7 +83,7 @@ def invoke_async_function(self, body: dict[str, Any]) -> None:
"""Invoke function asynchronously."""
url = self.get_conn().host + self.INVOKE_ASYNC_FUNCTION +
self.function_name
self.log.info("Invoking function asynchronously %s", url)
- response = requests.post(url, body)
+ response = requests.post(url, body, timeout=REQUEST_TIMEOUT)
Review Comment:
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.
##########
providers/openfaas/src/airflow/providers/openfaas/hooks/openfaas.py:
##########
@@ -71,7 +72,7 @@ def deploy_function(self, overwrite_function_if_exist: bool,
body: dict[str, Any
else:
url = self.get_conn().host + self.DEPLOY_FUNCTION
self.log.info("Deploying function %s", url)
- response = requests.post(url, body)
+ response = requests.post(url, body, timeout=REQUEST_TIMEOUT)
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
