Clean up bugbear issues (address #525)

servicex/app/cache.py

@@ -35,6 +35,8 @@
 from servicex.servicex_client import ServiceXClient
 
 cache_app = typer.Typer(name="cache", no_args_is_help=True)
+force_opt = typer.Option(False, "-y", help="Force, don't ask for permission")
+transform_id_arg = typer.Argument(help="Transform ID")
 
 
 @cache_app.callback()
@@ -73,9 +75,7 @@ def list():
 
 
 @cache_app.command()
-def clear(
-    force: bool = typer.Option(False, "-y", help="Force, don't ask for permission")
-):
+def clear(force: bool = force_opt):
     """
     Clear the local query cache
     """
@@ -87,7 +87,7 @@ def clear(
 
 
 @cache_app.command(no_args_is_help=True)
-def delete(transform_id: str = typer.Argument(help="Transform ID")):
+def delete(transform_id: str = transform_id_arg):
     """
     Delete a cached query. Use -t to specify the transform ID
     """

servicex/app/datasets.py

@@ -39,22 +39,26 @@
 from rich.table import Table
 
 datasets_app = typer.Typer(name="datasets", no_args_is_help=True)
+did_finder_opt = typer.Option(
+    None,
+    help="Filter datasets by DID finder. Some useful values are 'rucio' or 'user'",
+    show_default=False,
+)
+show_deleted_opt = typer.Option(
+    False,
+    help="Show deleted datasets",
+    show_default=True,
+)
+dataset_id_get_arg = typer.Argument(..., help="The ID of the dataset to get")
+dataset_id_delete_arg = typer.Argument(..., help="The ID of the dataset to delete")
 
 
 @datasets_app.command(no_args_is_help=False)
 def list(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    did_finder: Optional[str] = typer.Option(
-        None,
-        help="Filter datasets by DID finder. Some useful values are 'rucio' or 'user'",
-        show_default=False,
-    ),
-    show_deleted: Optional[bool] = typer.Option(
-        False,
-        help="Show deleted datasets",
-        show_default=True,
-    ),
+    did_finder: Optional[str] = did_finder_opt,
+    show_deleted: Optional[bool] = show_deleted_opt,
 ):
     """
     List the datasets. Use fancy formatting if printing to a terminal.
@@ -99,7 +103,7 @@ def list(
 def get(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    dataset_id: int = typer.Argument(..., help="The ID of the dataset to get"),
+    dataset_id: int = dataset_id_get_arg,
 ):
     """
     Get the details of a dataset. Output as a pretty, nested table if printing to a terminal.
@@ -143,7 +147,7 @@ def get(
 def delete(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    dataset_id: int = typer.Argument(..., help="The ID of the dataset to delete"),
+    dataset_id: int = dataset_id_delete_arg,
 ):
     sx = ServiceXClient(backend=backend, config_path=config_path)
     result = asyncio.run(sx.delete_dataset(dataset_id))

servicex/app/main.py

@@ -45,6 +45,11 @@
 app.add_typer(codegen_app)
 app.add_typer(datasets_app)
 
+spec_file_arg = typer.Argument(..., help="Spec file to submit to serviceX")
+ignore_cache_opt = typer.Option(
+    None, "--ignore-cache", help="Ignore local cache and always submit to ServiceX"
+)
+
 
 def show_version(show: bool):
     """Display the installed version and quit."""
@@ -53,12 +58,11 @@ def show_version(show: bool):
         raise typer.Exit()
 
 
+version_opt = typer.Option(None, "--version", callback=show_version, is_eager=True)
+
+
 @app.callback()
-def main_info(
-    version: Optional[bool] = typer.Option(
-        None, "--version", callback=show_version, is_eager=True
-    )
-):
+def main_info(version: Optional[bool] = version_opt):
     """
     ServiceX Client
     """
@@ -69,10 +73,8 @@ def main_info(
 def deliver(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    spec_file: str = typer.Argument(..., help="Spec file to submit to serviceX"),
-    ignore_cache: Optional[bool] = typer.Option(
-        None, "--ignore-cache", help="Ignore local cache and always submit to ServiceX"
-    ),
+    spec_file: str = spec_file_arg,
+    ignore_cache: Optional[bool] = ignore_cache_opt,
 ):
     """
     Deliver a file to the ServiceX cache.

servicex/app/transforms.py

@@ -44,6 +44,23 @@
 from servicex.servicex_client import ServiceXClient
 
 transforms_app = typer.Typer(name="transforms", no_args_is_help=True)
+complete_opt = typer.Option(
+    None, "--complete", help="Only show successfully completed transforms"
+)
+running_opt = typer.Option(
+    None, "--running", help="Only show transforms that are currently running"
+)
+transform_id_arg = typer.Argument(help="Transform ID")
+local_dir_opt = typer.Option(".", "-d", help="Local dir to download to")
+concurrency_opt = typer.Option(
+    20, "--concurrency", help="Number of concurrent downloads"
+)
+log_level_opt = typer.Option(
+    "ERROR", "-l", "--log-level", help="Level of Logs", case_sensitive=False
+)
+time_frame_opt = typer.Option(
+    "month", "-f", "--time-frame", help="Time Frame", case_sensitive=False
+)
 
 
 @transforms_app.callback()
@@ -58,12 +75,8 @@ def transforms():
 def list(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    complete: Optional[bool] = typer.Option(
-        None, "--complete", help="Only show successfully completed transforms"
-    ),
-    running: Optional[bool] = typer.Option(
-        None, "--running", help="Only show transforms that are currently running"
-    ),
+    complete: Optional[bool] = complete_opt,
+    running: Optional[bool] = running_opt,
 ):
     """
     List the transforms that have been run.
@@ -97,7 +110,7 @@ def transform_filter(status: Status) -> bool:
 def files(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    transform_id: str = typer.Argument(help="Transform ID"),
+    transform_id: str = transform_id_arg,
 ):
     """
     List the files that were produced by a transform.
@@ -123,11 +136,9 @@ async def list_files(sx: ServiceXClient, transform_id: str) -> List[ResultFile]:
 def download(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    transform_id: str = typer.Argument(help="Transform ID"),
-    local_dir: str = typer.Option(".", "-d", help="Local dir to download to"),
-    concurrency: int = typer.Option(
-        20, "--concurrency", help="Number of concurrent downloads"
-    ),
+    transform_id: str = transform_id_arg,
+    local_dir: str = local_dir_opt,
+    concurrency: int = concurrency_opt,
 ):
     """
     Download the files that were produced by a transform.
@@ -168,7 +179,7 @@ async def download_with_progress(filename) -> Path:
 def delete(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    transform_id_list: List[str] = typer.Argument(help="Transform ID"),
+    transform_id_list: List[str] = transform_id_arg,
 ):
     """
     Delete a completed transform along with the result files.
@@ -185,7 +196,7 @@ def delete(
 def cancel(
     backend: Optional[str] = backend_cli_option,
     config_path: Optional[str] = config_file_option,
-    transform_id_list: List[str] = typer.Argument(help="Transform ID"),
+    transform_id_list: List[str] = transform_id_arg,
 ):
     """
     Cancel a running transform request.
@@ -260,13 +271,9 @@ def create_kibana_link_parameters(
 @transforms_app.command(no_args_is_help=True)
 def logs(
     backend: Optional[str] = backend_cli_option,
-    transform_id: str = typer.Argument(help="Transform ID"),
-    log_level: Optional[LogLevel] = typer.Option(
-        "ERROR", "-l", "--log-level", help="Level of Logs", case_sensitive=False
-    ),
-    time_frame: Optional[TimeFrame] = typer.Option(
-        "month", "-f", "--time-frame", help="Time Frame", case_sensitive=False
-    ),
+    transform_id: str = transform_id_arg,
+    log_level: Optional[LogLevel] = log_level_opt,
+    time_frame: Optional[TimeFrame] = time_frame_opt,
 ):
     """
     Open the URL to the Kibana dashboard of the logs of a tranformer

tests/test_query_cache.py

@@ -235,8 +235,8 @@ def test_delete_codegen_by_backend_nonexistent():
         config = Configuration(cache_path=temp_dir, api_endpoints=[])  # type: ignore
         cache = QueryCache(config)
         cache.delete_codegen_by_backend("backend_1")
-        with pytest.raises(Exception):
-            assert False
+        with pytest.raises(AssertionError):
+            raise AssertionError()
         cache.close()