Merge pull request #34304 Better error messages for unavailable providers.

Author: robertwb

sdks/python/apache_beam/yaml/yaml_provider.py

@@ -37,6 +37,7 @@
 from collections.abc import Mapping
 from typing import Any
 from typing import Optional
+from typing import Union
 
 import docstring_parser
 import yaml
@@ -65,9 +66,21 @@
 from apache_beam.yaml.yaml_errors import maybe_with_exception_handling_transform_fn
 
 
+class NotAvailableWithReason:
+  """A False value that provides additional content.
+
+  Primarily used to return a value from Provider.available().
+  """
+  def __init__(self, reason):
+    self.reason = reason
+
+  def __bool__(self):
+    return False
+
+
 class Provider:
   """Maps transform types names and args to concrete PTransform instances."""
-  def available(self) -> bool:
+  def available(self) -> Union[bool, NotAvailableWithReason]:
     """Returns whether this provider is available to use in this environment."""
     raise NotImplementedError(type(self))
 
@@ -308,6 +321,7 @@ class RemoteProvider(ExternalProvider):
 
   def __init__(self, urns, address: str):
     super().__init__(urns, service=address)
+    self._address = address
 
   def available(self):
     if self._is_available is None:
@@ -316,7 +330,8 @@ def available(self):
           service.ready(1)
           self._is_available = True
       except Exception:
-        self._is_available = False
+        self._is_available = NotAvailableWithReason(
+            f'Remote provider not reachable at {self._address}.')
     return self._is_available
 
   def cache_artifacts(self):
@@ -331,8 +346,20 @@ def __init__(self, urns, jar_provider):
 
   def available(self):
     # pylint: disable=subprocess-run-check
-    return subprocess.run(['which', 'java'],
-                          capture_output=True).returncode == 0
+    trial = subprocess.run(['which', 'java'], capture_output=True)
+    if trial.returncode == 0:
+      return True
+    else:
+
+      def try_decode(bs):
+        try:
+          return bs.decode()
+        except UnicodeError:
+          return bs
+
+      return NotAvailableWithReason(
+          f'Unable to locate java executable: '
+          f'{try_decode(trial.stdout)}{try_decode(trial.stderr)}')
 
   def cache_artifacts(self):
     return [self._jar_provider()]

sdks/python/apache_beam/yaml/yaml_transform.py

@@ -237,13 +237,26 @@ def best_provider(
       spec = t
     else:
       spec = self._transforms_by_uuid[self.get_transform_id(t)]
-    possible_providers = [
-        p for p in self.providers[spec['type']] if p.available()
-    ]
+    possible_providers = []
+    unavailable_provider_messages = []
+    for p in self.providers[spec['type']]:
+      is_available = p.available()
+      if is_available:
+        possible_providers.append(p)
+      else:
+        reason = getattr(is_available, 'reason', 'no reason given')
+        unavailable_provider_messages.append(
+            f'{p.__class__.__name__} ({reason})')
     if not possible_providers:
+      if unavailable_provider_messages:
+        unavailable_provider_message = (
+            '\nThe following providers were found but not available: ' +
+            '\n'.join(unavailable_provider_messages))
+      else:
+        unavailable_provider_message = ''
       raise ValueError(
-          'No available provider for type %r at %s' %
-          (spec['type'], identify_object(spec)))
+          'No available provider for type %r at %s%s' %
+          (spec['type'], identify_object(spec), unavailable_provider_message))
     # From here on, we have the invariant that possible_providers is not empty.
 
     # Only one possible provider, no need to rank further.

sdks/python/apache_beam/yaml/yaml_transform_test.py

@@ -534,7 +534,7 @@ def test_strip_error_metadata(self):
               config:
                   language: python
                   fields:
-                    out: "1/(1-len(element))"
+                    out: "1.0/(1-len(element))"
                   error_handling:
                     output: errors
             - type: StripErrorMetadata