SNOW-2204396: Applied platform_detection.py changes

Author: sfc-gh-fpawlowski

src/snowflake/connector/platform_detection.py

@@ -30,7 +30,8 @@ class _DetectionState(Enum):
 
     DETECTED = "detected"
     NOT_DETECTED = "not_detected"
-    TIMEOUT = "timeout"
+    HTTP_TIMEOUT = "timeout"
+    WORKER_TIMEOUT = "worker_timeout"
 
 
 # Result returned when platform detection is disabled via environment variable
@@ -157,7 +158,7 @@ def is_azure_vm(
         session_manager: SessionManager instance for making HTTP requests.
 
     Returns:
-        _DetectionState: DETECTED if on Azure VM, TIMEOUT if request times out,
+        _DetectionState: DETECTED if on Azure VM, HTTP_TIMEOUT if request times out,
                         NOT_DETECTED otherwise.
     """
     try:
@@ -172,7 +173,7 @@ def is_azure_vm(
             else _DetectionState.NOT_DETECTED
         )
     except Timeout:
-        return _DetectionState.TIMEOUT
+        return _DetectionState.HTTP_TIMEOUT
     except RequestException:
         return _DetectionState.NOT_DETECTED
 
@@ -219,7 +220,7 @@ def is_managed_identity_available_on_azure_vm(
         resource: The Azure resource URI to request a token for.
 
     Returns:
-        _DetectionState: DETECTED if managed identity is available, TIMEOUT if request
+        _DetectionState: DETECTED if managed identity is available, HTTP_TIMEOUT if request
                         times out, NOT_DETECTED otherwise.
     """
     endpoint = f"http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource={resource}"
@@ -234,7 +235,7 @@ def is_managed_identity_available_on_azure_vm(
             else _DetectionState.NOT_DETECTED
         )
     except Timeout:
-        return _DetectionState.TIMEOUT
+        return _DetectionState.HTTP_TIMEOUT
     except RequestException:
         return _DetectionState.NOT_DETECTED
 
@@ -261,7 +262,7 @@ def has_azure_managed_identity(
         session_manager: SessionManager instance for making HTTP requests.
 
     Returns:
-        _DetectionState: DETECTED if managed identity is available, TIMEOUT if
+        _DetectionState: DETECTED if managed identity is available, HTTP_TIMEOUT if
                         detection timed out, NOT_DETECTED otherwise.
     """
     # short circuit early to save on latency and avoid minting an unnecessary token
@@ -290,7 +291,7 @@ def is_gce_vm(
         session_manager: SessionManager instance for making HTTP requests.
 
     Returns:
-        _DetectionState: DETECTED if on GCE, TIMEOUT if request times out,
+        _DetectionState: DETECTED if on GCE, HTTP_TIMEOUT if request times out,
                         NOT_DETECTED otherwise.
     """
     try:
@@ -304,7 +305,7 @@ def is_gce_vm(
             else _DetectionState.NOT_DETECTED
         )
     except Timeout:
-        return _DetectionState.TIMEOUT
+        return _DetectionState.HTTP_TIMEOUT
     except RequestException:
         return _DetectionState.NOT_DETECTED
 
@@ -360,7 +361,7 @@ def has_gcp_identity(
         platform_detection_timeout_seconds: Timeout value for the metadata service request.
         session_manager: SessionManager instance for making HTTP requests.
     Returns:
-        _DetectionState: DETECTED if valid GCP identity exists, TIMEOUT if request
+        _DetectionState: DETECTED if valid GCP identity exists, HTTP_TIMEOUT if request
                         times out, NOT_DETECTED otherwise.
     """
     try:
@@ -375,7 +376,7 @@ def has_gcp_identity(
             else _DetectionState.NOT_DETECTED
         )
     except Timeout:
-        return _DetectionState.TIMEOUT
+        return _DetectionState.HTTP_TIMEOUT
     except RequestException:
         return _DetectionState.NOT_DETECTED
 
@@ -412,11 +413,11 @@ def detect_platforms(
         session_manager: SessionManager instance for making HTTP requests. If None, a new instance will be created.
 
     Returns:
-        list[str]: List of detected platform names. Platforms that timed out will have
-                  "_timeout" suffix appended to their name. Returns _PLATFORM_DETECTION_DISABLED_RESULT
-                  if the ENV_VAR_DISABLE_PLATFORM_DETECTION environment variable is set to a value
-                  in ENV_VAR_BOOL_POSITIVE_VALUES_LOWERCASED (case-insensitive).
-                  Returns empty list if any exception occurs during detection.
+        list[str]: List of detected platform names. Platforms that timed out (either HTTP timeout
+                  or thread timeout) will have "_timeout" suffix appended to their name.
+                  Returns _PLATFORM_DETECTION_DISABLED_RESULT if the ENV_VAR_DISABLE_PLATFORM_DETECTION
+                  environment variable is set to a value in ENV_VAR_BOOL_POSITIVE_VALUES_LOWERCASED
+                  (case-insensitive). Returns empty list if any exception occurs during detection.
     """
     try:
         # Check if platform detection is disabled via environment variable
@@ -491,15 +492,20 @@ def detect_platforms(
                             timeout=platform_detection_timeout_seconds
                         )
                     except (FutureTimeoutError, FutureCancelledError):
-                        platforms[key] = _DetectionState.TIMEOUT
+                        # Thread/future timed out at executor level
+                        platforms[key] = _DetectionState.WORKER_TIMEOUT
                     except Exception:
+                        # Any other error from the thread
                         platforms[key] = _DetectionState.NOT_DETECTED
 
         detected_platforms = []
         for platform_name, detection_state in platforms.items():
             if detection_state == _DetectionState.DETECTED:
                 detected_platforms.append(platform_name)
-            elif detection_state == _DetectionState.TIMEOUT:
+            elif detection_state in (
+                _DetectionState.HTTP_TIMEOUT,
+                _DetectionState.WORKER_TIMEOUT,
+            ):
                 detected_platforms.append(f"{platform_name}_timeout")
 
         logger.debug(

test/unit/test_detect_platforms.py

@@ -65,7 +65,9 @@ def teardown(self):
     def test_no_platforms_detected(
         self, unavailable_metadata_service_with_request_exception
     ):
-        result = detect_platforms(platform_detection_timeout_seconds=None)
+        result = detect_platforms(
+            platform_detection_timeout_seconds=0.3
+        )  # increase timeout to make sure no Thread-based timeout messes the results
         assert result == []
 
     def test_ec2_instance_detection(
@@ -407,7 +409,7 @@ def capture_timeout_azure(timeout, session_manager):
             "snowflake.connector.platform_detection.is_azure_vm",
             side_effect=capture_timeout_azure,
         ):
-            detect_platforms()
+            detect_platforms(platform_detection_timeout_seconds=None)
 
             # Verify that functions were called with timeout <= 200ms
             assert len(timeout_captured) > 0, "No timeout was captured"
@@ -434,9 +436,9 @@ def test_platform_detection_completes_within_timeout(
         # Allow ~30% overhead for thread management, environment variable checks, etc.
         # The timeout is 200ms per network call, but they run in parallel
         # So total time should be ~200ms + overhead, not 200ms * number_of_calls
-        epsilon_for_overhead = 1.3
-        max_allowed_time = (
-            EXPECTED_MAX_TIMEOUT_FOR_PLATFORM_DETECTION * epsilon_for_overhead
+        epsilon_for_overhead = 0.3
+        max_allowed_time = EXPECTED_MAX_TIMEOUT_FOR_PLATFORM_DETECTION * (
+            1 + epsilon_for_overhead
         )
         assert execution_time < max_allowed_time, (
             f"Platform detection took {execution_time:.3f}s, "
@@ -445,5 +447,5 @@ def test_platform_detection_completes_within_timeout(
         # Ensure it's not suspiciously fast (< 10ms would indicate something's wrong)
         assert execution_time > 0.01, (
             f"Platform detection completed too quickly ({execution_time:.3f}s), "
-            "which may indicate detection was skipped"
+            "which may indicate detection was skipped or some other issues happened"
         )