You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Ironankit525 (via GitHub)" <gi...@apache.org> on 2026/03/07 17:01:09 UTC

[PR] Add missing HTTP timeouts across multiple providers [airflow]

Ironankit525 opened a new pull request, #63042:
URL: https://github.com/apache/airflow/pull/63042

   This PR adds a default timeout of 30 seconds to several HTTP requests across multiple providers that were missing it. This prevents tasks from hanging indefinitely in case of network issues or unresponsive servers.
   
   ### Affected providers:
   - OpenFaaS (`OpenFaasHook`)
   - - Apache Druid (`DruidHook`)
   - - FAB (`FabAirflowSecurityManagerOverride`)
   - - CNCF Kubernetes (`KubernetesHook`)
   - - Google Cloud Dataprep (`DataprepHook`)
   Closes: #63033


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add missing HTTP timeouts across multiple providers [airflow]

Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #63042:
URL: https://github.com/apache/airflow/pull/63042#issuecomment-4017138119

   For example; what if I have my Task-level `execution_timeout` set for 5 minutes. If I have a request `timeout` of 30 seconds, we could have an issue.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add missing HTTP timeouts across multiple providers [airflow]

Posted by "Ironankit525 (via GitHub)" <gi...@apache.org>.
Ironankit525 commented on PR #63042:
URL: https://github.com/apache/airflow/pull/63042#issuecomment-4017275506

   Thanks for the thorough review, @jroachgolf84 and @jscheffl! I completely agree that adding blanket timeouts without justification can be risky, especially for methods that might block for long-running operations.
   
   I will close this combined PR and open separate, per-provider PRs that include proper justification, configuration options where necessary, and only apply timeouts to control-plane/metadata operations rather than blocking execution tasks.
   
   @jscheffl regarding the Kubernetes Hook get_yaml_content_from_file: this method actually uses requests.get to fetch a raw YAML file from a generic URL (like GitHub) rather than hitting the K8s API server via the kubernetes Python client. That's why the timeout is applied here directly to requests.get. I'll address this in a separate focused PR.
   
   Closing this PR in favor of splitting.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add missing HTTP timeouts across multiple providers [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #63042:
URL: https://github.com/apache/airflow/pull/63042#discussion_r2900344440


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -778,7 +778,7 @@ def check_kueue_deployment_running(
     @staticmethod
     def get_yaml_content_from_file(kueue_yaml_url) -> list[dict]:
         """Download content of YAML file and separate it into several dictionaries."""
-        response = requests.get(kueue_yaml_url, allow_redirects=True)
+        response = requests.get(kueue_yaml_url, allow_redirects=True, timeout=30)

Review Comment:
   I'd be okay to apply 30s timeout across K8s provider as usually all calls should be short. But this here is a simple search&add. The K8s provider uses a K8s API Client, is a timeout should be applied (and is not already) then it needs to be passed down to API client (where a generic retry layer was added recently)



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add missing HTTP timeouts across multiple providers [airflow]

Posted by "Ironankit525 (via GitHub)" <gi...@apache.org>.
Ironankit525 closed pull request #63042: Add missing HTTP timeouts across multiple providers
URL: https://github.com/apache/airflow/pull/63042


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org