Merge pull request #124 from ssl-hep:gordonwatts/issue123

- Make sure the image name is None by default
- Do not include an "image" tag in the API request if we don't need it.
- Fix up some flake8 errors

Fixes #123

Author: gordonwatts

servicex/servicex.py

@@ -38,7 +38,7 @@ class ServiceXDataset(ServiceXABC):
     def __init__(self,
                  dataset: str,
                  backend_type: Optional[str] = None,
-                 image: str = 'sslhep/servicex_func_adl_xaod_transformer:v1.0.0-rc.2',  # NOQA
+                 image: str = None,
                  max_workers: int = 20,
                  servicex_adaptor: ServiceXAdaptor = None,
                  minio_adaptor: Union[MinioAdaptor, MinioAdaptorFactory] = None,
@@ -60,7 +60,9 @@ def __init__(self,
                                         will default to xaod, unless you have any endpoint listed
                                         in your servicex file. It will default to best match there,
                                         in that case.
-            image                       Name of transformer image to use to transform the data
+            image                       Name of transformer image to use to transform the data. If
+                                        left as default, `None`, then the default image for the
+                                        ServiceX backend will be used.
             max_workers                 Maximum number of transformers to run simultaneously on
                                         ServiceX.
             servicex_adaptor            Object to control communication with the servicex instance
@@ -83,8 +85,8 @@ def __init__(self,
                                         and `awkward`, including default settings for expected
                                         datatypes from the backend.
             ignore_cache                Always ignore the cache on any query for this dataset. This
-                                        is only meaningful if no cache adaptor is provided. Defaults
-                                        to false - the cache is used if possible.
+                                        is only meaningful if no cache adaptor is provided.
+                                        Defaults to false - the cache is used if possible.
 
         Notes:
 
@@ -95,6 +97,8 @@ def __init__(self,
                takes `(total_files, transformed, downloaded, skipped)` as an argument. The
                `total_files` parameter may be `None` until the system knows how many files need to
                be processed (and some files can even be completed before that is known).
+            -  The full description of calling parameters is recorded in the local cache, including
+               things like `image` name and tag.
         '''
         ServiceXABC.__init__(self, dataset, image, max_workers,
                              status_callback_factory,
@@ -414,16 +418,20 @@ def _build_json_query(self, selection_query: str, data_type: str) -> Dict[str, s
         Notes:
             - Internal routine.
         '''
+        # Items that must always be present
         json_query: Dict[str, str] = {
             "did": self._dataset,
             "selection": selection_query,
-            "image": self._image,
             "result-destination": "object-store",
             "result-format": 'parquet' if data_type == 'parquet' else "root-file",
             "chunk-size": '1000',
             "workers": str(self._max_workers)
         }
 
+        # Optional items
+        if self._image is not None:
+            json_query['image'] = self._image
+
         logging.getLogger(__name__).debug(f'JSON to be sent to servicex: {str(json_query)}')
 
         return json_query

servicex/servicex_config.py

@@ -91,7 +91,7 @@ def get_servicex_adaptor_config(self, backend_type: Optional[str] = None) -> \
 
         Returns:
             Tuple[str, str]: The tuple of info to create a `ServiceXAdaptor`: end point,
-            token.
+            token (optionally).
         '''
         # Find a list of all endpoints.
         # It is an error if this is not specified somewhere.

servicex/servicexabc.py

@@ -27,7 +27,7 @@ class ServiceXABC(ABC):
 
     def __init__(self,
                  dataset: str,
-                 image: str = 'sslhep/servicex_func_adl_xaod_transformer:v0.4',
+                 image: Optional[str] = None,
                  max_workers: int = 20,
                  status_callback_factory: Optional[StatusUpdateFactory] = _run_default_wrapper,
                  ):
@@ -37,7 +37,8 @@ def __init__(self,
         Arguments
 
             dataset                     Name of a dataset from which queries will be selected.
-            image                       Name of transformer image to use to transform the data
+            image                       Name of transformer image to use to transform the data. If
+                                        None the default implementation is used.
             cache_adaptor               Runs the caching for data and queries that are sent up and
                                         down.
             max_workers                 Maximum number of transformers to run simultaneously on

tests/test_servicex.py

@@ -79,7 +79,8 @@ def test_default_ctor_cache_no(mocker):
 
     config = mocker.MagicMock(spec=ServiceXConfigAdaptor)
     config.settings = Configuration('servicex', 'servicex')
-    config.get_servicex_adaptor_config.return_value = ('http://no-way.dude', 'j@1232j322432j22token.com')
+    config.get_servicex_adaptor_config.return_value = ('http://no-way.dude',
+                                                       'j@1232j322432j22token.com')
 
     cache = mocker.MagicMock(spec=Cache)
     cache_create_call = mocker.patch('servicex.servicex.Cache', return_value=cache)
@@ -98,6 +99,7 @@ def test_ignore_cache_on_ds(mocker):
     config.get_servicex_adaptor_config.return_value = ('http://no-way.dude', '1232j322432j22token')
 
     got_called = False
+
     @contextmanager
     def do_context():
         nonlocal got_called
@@ -432,6 +434,25 @@ async def test_image_spec(mocker, good_awkward_file_data):
     assert called['image'] == 'fork-it-over:latest'
 
 
+@pytest.mark.asyncio
+async def test_no_image_spec(mocker, good_awkward_file_data):
+    mock_cache = build_cache_mock(mocker)
+    mock_logger = mocker.MagicMock(spec=log_adaptor)
+    mock_servicex_adaptor = MockServiceXAdaptor(mocker, "123-456")
+    mock_minio_adaptor = MockMinioAdaptor(mocker, files=['one_minio_entry'])
+
+    ds = fe.ServiceXDataset('localds://mc16_tev:13',
+                            servicex_adaptor=mock_servicex_adaptor,  # type: ignore
+                            minio_adaptor=mock_minio_adaptor,  # type: ignore
+                            cache_adaptor=mock_cache,
+                            local_log=mock_logger,
+                            data_convert_adaptor=good_awkward_file_data)
+    await ds.get_data_rootfiles_async('(valid qastle string)')
+
+    called = mock_servicex_adaptor.query_json
+    assert 'image' not in called
+
+
 @pytest.mark.asyncio
 async def test_max_workers_spec(mocker, good_awkward_file_data):
     mock_cache = build_cache_mock(mocker)