Remove minio config info from the servicex file (#140)

Fixes #108

Author: gordonwatts

README.md

@@ -33,7 +33,7 @@ The API access information is normally placed in a `.servicex` file (to keep thi
    directory on Windows).
 1. The `config_defaults.yaml` file distributed with the `servicex` package.
 
-If no endpoint is specified, then the library defaults to the developer endpoint, which is `http://localhost:5000` for the web-service API, and `localhost:9000` for the `minio` endpoint. No passwords are required.
+If no endpoint is specified, then the library defaults to the developer endpoint, which is `http://localhost:5000` for the web-service API. No passwords are used in this case.
 
 Create a `.servicex` file, in the `yaml` format, in the appropriate place for your work that contains the following (for the `xaod` backend; use `uproot` for the uproot backend):
 
@@ -151,7 +151,6 @@ The file can contain an `api_endpoint` as mentioned above. In addition the other
   - Windows: `cache_path: "C:\\Users\\gordo\\Desktop\\cacheme"`
   - Linux: `cache_path: "/home/servicex-cache"`
 
-- `minio_endpoint`, `minio_username`, `minio_password` - these are only interesting if you are using a pre-RC2 release of `servicex` - when the `minio` information wasn't part of the API exchange. This feature is depreciated and will be removed around the time `servicex` moves to RC3.
 - `backend_types` - a list of yaml dictionaries that contains some defaults for the backends. By default only the `return_data` is there, which for `xaod` is `root` and `uproot` is `parquet`. Allows `servicex` to convert to `pandas.DataFrame` or `awkward` if requested by the user.
 
 All strings are expanded using python's [os.path.expand](https://docs.python.org/3/library/os.path.html#os.path.expandvars) method - so `$NAME` and `${NAME}` will work to expand existing environment variables.

servicex/config_default.yaml

@@ -5,20 +5,6 @@ api_endpoints:
   - endpoint: http://localhost:5000
     # token: xxx
 
-# These are default settings, depreciated, and should not be used.
-# They will be removed in the next version.
-api_endpoint:
-  minio_endpoint: localhost:9000
-  # The username and password for accessing files generated by servicex.
-  # NOTE:
-  #  1. If minio_username and password are specified they will be used.
-  #  2. If only an endpoint username password is specified they will be used.
-  #  3. If nothing is specified, then the default_xxxx username passwords will be used.
-  # minio_password: xxxx
-  # minio_username: xxxx
-  default_minio_username: miniouser
-  default_minio_password: leftfoot1
-
 # This is the path of the cache. The "/tmp" will be translated, platform appropriate.
 cache_path: /tmp/servicex
 

servicex/minio_adaptor.py

@@ -28,12 +28,11 @@
 import asyncio
 from concurrent.futures.thread import ThreadPoolExecutor
 from pathlib import Path
-from typing import Any, AsyncIterator, Optional, cast, Dict
+from typing import Any, AsyncIterator, List, Optional, Dict, cast
 import logging
 
 import backoff
 from backoff import on_exception
-from confuse import ConfigView
 from minio import Minio, ResponseError
 
 from .utils import ServiceXException
@@ -59,8 +58,8 @@ def __init__(self, minio_endpoint: str,
                              secure=self._secured)
 
     @on_exception(backoff.constant, ResponseError, interval=0.1)
-    def get_files(self, request_id):
-        return [f.object_name for f in self._client.list_objects(request_id)]
+    def get_files(self, request_id) -> List[str]:
+        return [str(f.object_name) for f in self._client.list_objects(request_id)]
 
     async def download_file(self,
                             request_id: str,
@@ -106,8 +105,7 @@ class MinioAdaptorFactory:
     '''A factor that will return, when asked, the proper minio adaptor to use for a request
     to get files from ServiceX.
     '''
-    def __init__(self, c: Optional[ConfigView] = None,
-                 always_return: Optional[MinioAdaptor] = None):
+    def __init__(self, always_return: Optional[MinioAdaptor] = None):
         '''Create the factor with a possible adaptor to always use
 
         Args:
@@ -117,8 +115,6 @@ def __init__(self, c: Optional[ConfigView] = None,
         # Get the defaults setup.
         self._always = always_return
         self._config_adaptor = None
-        if self._always is None and c is not None:
-            self._config_adaptor = self._from_config(c)
 
     def from_best(self, transaction_info: Optional[Dict[str, str]] = None) -> MinioAdaptor:
         '''Using the information we have, create the proper Minio Adaptor with the correct
@@ -143,41 +139,11 @@ def from_best(self, transaction_info: Optional[Dict[str, str]] = None) -> MinioA
             if all(k in transaction_info for k in keys):
                 logging.getLogger(__name__).debug('Using the request-specific minio_adaptor')
                 return MinioAdaptor(transaction_info['minio-endpoint'],
-                                    transaction_info['minio-secured'],
+                                    bool(transaction_info['minio-secured']),
                                     transaction_info['minio-access-key'],
                                     transaction_info['minio-secret-key'])
-        if self._config_adaptor is not None:
-            logging.getLogger(__name__).debug('Using the config-file minio_adaptor')
-            return self._config_adaptor
-        raise ServiceXException("Do not know how to create a Minio Login info")
-
-    def _from_config(self, c: ConfigView) -> MinioAdaptor:
-        '''Extract the Minio config information from the config file(s). This will be used
-        if minio login information isn't returned from the request.
-
-        Args:
-            c (ConfigView): The loaded config
-
-        Returns:
-            MinioAdaptor: The adaptor that uses the config's login information.
-        '''
-        c_api = c['api_endpoint']
-        end_point = cast(str, c_api['minio_endpoint'].as_str_expanded())
-
-        # Grab the username and password if they are explicitly listed.
-        if 'minio_username' in c_api:
-            username = c_api['minio_username'].as_str_expanded()
-            password = c_api['minio_password'].as_str_expanded()
-        elif 'username' in c_api:
-            username = c_api['username'].as_str_expanded()
-            password = c_api['password'].as_str_expanded()
-        else:
-            username = c_api['default_minio_username'].as_str_expanded()
-            password = c_api['default_minio_password'].as_str_expanded()
-
-        return MinioAdaptor(end_point,
-                            access_key=cast(str, username),
-                            secretkey=cast(str, password))
+        raise ServiceXException("Do not know or have enough information to create a Minio "
+                                f"Login info ({transaction_info})")
 
 
 async def find_new_bucket_files(adaptor: MinioAdaptor,
@@ -190,7 +156,7 @@ async def find_new_bucket_files(adaptor: MinioAdaptor,
     seen = []
     async for _ in update:
         # Sadly, this is blocking, and so may hold things up
-        files = adaptor.get_files(request_id)
+        files = cast(List[str], adaptor.get_files(request_id))
 
         # If there are new files, pass them on
         for f in files:

tests/test_minio_adaptor.py

@@ -134,15 +134,15 @@ def test_list_objects(good_minio_client):
     ma = MinioAdaptor('localhost:9000')
 
     f = ma.get_files('111-222-333-444')
-    assert len(f) == 1
+    assert len(f) == 1  # type: ignore
 
 
 def test_list_objects_with_null(bad_then_good_minio_listing):
     '''Sometimes for reasons we do not understand we get back a response error from
     list_objects minio method'''
     ma = MinioAdaptor('localhost:9000')
     f = ma.get_files('111-222-333-444')
-    assert len(f) == 1
+    assert len(f) == 1  # type: ignore
 
 
 @pytest.mark.asyncio
@@ -178,55 +178,6 @@ def test_factor_always():
     assert f.from_best() is a
 
 
-def test_factory_set_endpoint():
-    from confuse import Configuration
-    c = Configuration('bogus', 'bogus')
-    c.clear()
-    c['api_endpoint']['minio_endpoint'] = 'the-good-host.org:9000'
-    c['api_endpoint']['minio_username'] = 'amazing'
-    c['api_endpoint']['minio_password'] = 'forkingshirtballs'
-
-    c['api_endpoint']['default_minio_username'] = 'badnews'
-    c['api_endpoint']['default_minio_password'] = 'bears'
-
-    m = MinioAdaptorFactory(c).from_best()
-    assert m._endpoint == 'the-good-host.org:9000'
-    assert m._access_key == "amazing"
-    assert m._secretkey == "forkingshirtballs"
-
-
-def test_factory_use_api_usernamepassword():
-    from confuse import Configuration
-    c = Configuration('bogus', 'bogus')
-    c.clear()
-
-    c['api_endpoint']['endpoint'] = 'http://my-left-foot.com:5000'
-    c['api_endpoint']['username'] = 'thegoodplace'
-    c['api_endpoint']['password'] = 'forkingshirtballs!'
-
-    c['api_endpoint']['minio_endpoint'] = 'the-good-host.org:9000'
-
-    m = MinioAdaptorFactory(c).from_best()
-    assert m._endpoint == 'the-good-host.org:9000'
-    assert m._access_key == "thegoodplace"
-    assert m._secretkey == "forkingshirtballs!"
-
-
-def test_factory_use_default_username_password():
-    from confuse import Configuration
-    c = Configuration('bogus', 'bogus')
-    c.clear()
-
-    c['api_endpoint']['minio_endpoint'] = 'the-good-host.org:9000'
-    c['api_endpoint']['default_minio_username'] = 'thegoodplace'
-    c['api_endpoint']['default_minio_password'] = 'forkingshirtballs!'
-
-    m = MinioAdaptorFactory(c).from_best()
-    assert m._endpoint == 'the-good-host.org:9000'
-    assert m._access_key == "thegoodplace"
-    assert m._secretkey == "forkingshirtballs!"
-
-
 def test_factory_from_request():
     info = {
         'minio-access-key': 'miniouser',
@@ -239,19 +190,3 @@ def test_factory_from_request():
     assert not m._secured
     assert m._access_key == "miniouser"
     assert m._secretkey == "leftfoot1"
-
-
-def test_factory_request_missing():
-    info = {}
-    from confuse import Configuration
-    c = Configuration('bogus', 'bogus')
-    c.clear()
-
-    c['api_endpoint']['minio_endpoint'] = 'the-good-host.org:9000'
-    c['api_endpoint']['default_minio_username'] = 'thegoodplace'
-    c['api_endpoint']['default_minio_password'] = 'forkingshirtballs!'
-
-    m = MinioAdaptorFactory(c).from_best(info)
-    assert m._endpoint == 'the-good-host.org:9000'
-    assert m._access_key == "thegoodplace"
-    assert m._secretkey == "forkingshirtballs!"