Improve invalid backend error (#626)

* Enhance backend error message

* Add config file info to backend errors (#627)

* Improve backend error message

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add config test to make sure full coverage

* Address review comment - missed saving resolve!

Author: gordonwatts

servicex/configuration.py

@@ -49,6 +49,10 @@ class Configuration(BaseModel):
     )
 
     shortened_downloaded_filename: Optional[bool] = False
+    # Path to the configuration file this object was read from. This field is
+    # populated by :py:meth:`Configuration.read` and is not part of the input
+    # schema.
+    config_file: Optional[str] = Field(default=None, exclude=True)
 
     @model_validator(mode="after")
     def expand_cache_path(self):
@@ -96,12 +100,17 @@ def read(cls, config_path: Optional[str] = None):
         :return: Populated configuration object
         """
         if config_path:
-            yaml_config = cls._add_from_path(Path(config_path), walk_up_tree=False)
+            yaml_config, cfg_path = cls._add_from_path(
+                Path(config_path), walk_up_tree=False
+            )
         else:
-            yaml_config = cls._add_from_path(walk_up_tree=True)
+            yaml_config, cfg_path = cls._add_from_path(walk_up_tree=True)
 
         if yaml_config:
-            return Configuration.model_validate(yaml_config)
+            cfg = Configuration.model_validate(yaml_config)
+            if cfg_path:
+                cfg.config_file = str(cfg_path)
+            return cfg
         else:
             path_extra = f"in {config_path}" if config_path else ""
             raise NameError(
@@ -111,8 +120,9 @@ def read(cls, config_path: Optional[str] = None):
     @classmethod
     def _add_from_path(cls, path: Optional[Path] = None, walk_up_tree: bool = False):
         config = None
+        found_file: Optional[Path] = None
         if path:
-            path.resolve()
+            path = path.resolve()
             name = path.name
             dir = path.parent.resolve()
             alt_name = None
@@ -126,14 +136,16 @@ def _add_from_path(cls, path: Optional[Path] = None, walk_up_tree: bool = False)
             if f.exists():
                 with open(f) as config_file:
                     config = yaml.safe_load(config_file)
-                    break
+                found_file = f
+                break
 
             if alt_name:
                 f = dir / alt_name  # if neither option above, find servicex.yaml
                 if f.exists():
                     with open(f) as config_file:
                         config = yaml.safe_load(config_file)
-                        break
+                    found_file = f
+                    break
 
             if not walk_up_tree:
                 break
@@ -155,6 +167,7 @@ def _add_from_path(cls, path: Optional[Path] = None, walk_up_tree: bool = False)
                 if f.exists():
                     with open(f) as config_file:
                         config = yaml.safe_load(config_file)
+                        found_file = f
                     break
 
-        return config
+        return config, found_file

servicex/servicex_client.py

@@ -343,7 +343,12 @@ def __init__(self, backend=None, url=None, config_path=None):
             self.servicex = ServiceXAdapter(url)
         elif backend:
             if backend not in self.endpoints:
-                raise ValueError(f"Backend {backend} not defined in .servicex file")
+                valid_backends = ", ".join(self.endpoints.keys())
+                cfg_file = self.config.config_file or ".servicex"
+                raise ValueError(
+                    f"Backend {backend} not defined in {cfg_file} file. "
+                    f"Valid backend names: {valid_backends}"
+                )
             self.servicex = ServiceXAdapter(
                 self.endpoints[backend].endpoint,
                 refresh_token=self.endpoints[backend].token,

tests/test_config.py

@@ -92,3 +92,25 @@ def test_read_from_home(monkeypatch, tmp_path):
 
     c = Configuration.read()
     assert c.api_endpoints[0].endpoint == "http://localhost:5000"
+
+
+@pytest.mark.parametrize("config_filename", ["servicex.yaml", ".servicex"])
+def test_read_from_default_files(monkeypatch, tmp_path, config_filename):
+    """
+    Ensure config can be located in the user's home directory for servicex.yaml and .servicex.
+    """
+
+    # Create a fake home directory with the config file
+    cfg = tmp_path / config_filename
+    cfg.write_text(
+        """
+api_endpoints:
+  - endpoint: http://localhost:5012
+    name: localhost
+"""
+    )
+
+    monkeypatch.chdir(tmp_path)
+
+    c = Configuration.read()
+    assert c.api_endpoints[0].endpoint == "http://localhost:5012"

tests/test_servicex_client.py

@@ -26,6 +26,7 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 from datetime import datetime
+from pathlib import Path
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -138,3 +139,13 @@ def test_delete_transform_from_cache(mock_cache, servicex_adaptor, transformed_r
             mock_cache.return_value.delete_record_by_request_id.assert_called_once_with(
                 "servicex-request-789"
             )
+
+
+def test_invalid_backend_raises_error_with_filename():
+    config_file = "tests/example_config.yaml"
+    expected = Path(config_file).resolve()
+
+    with pytest.raises(ValueError) as err:
+        ServiceXClient(backend="badname", config_path=config_file)
+
+    assert f"Backend badname not defined in {expected} file" in str(err.value)