diff --git a/lib/pbench/server/api/resources/datasets_compare.py b/lib/pbench/server/api/resources/datasets_compare.py index 6f08c6557f..f123a17451 100644 --- a/lib/pbench/server/api/resources/datasets_compare.py +++ b/lib/pbench/server/api/resources/datasets_compare.py @@ -21,7 +21,7 @@ Schema, ) from pbench.server.cache_manager import CacheManager -from pbench.server.database.models.datasets import Metadata +from pbench.server.database.models.datasets import Dataset, Metadata class DatasetsCompare(ApiBase): @@ -29,6 +29,35 @@ class DatasetsCompare(ApiBase): This class implements the Server API used to retrieve comparison data for visualization. """ + @staticmethod + def get_benchmark_name(dataset: Dataset) -> str: + """Convenience function to get dataset's benchmark + + The Pbench Server intake constructs a server.benchmark metadata key to + represent the benchmark type. This helper implements a fallback for + datasets processed prior to the implementation of server.benchmark to + avoid null values, by using the Pbench Agent metadata.log "script" + value if that exists and then a constant fallback value. + + TODO: This is a workaround from the transition to server.benchmark in + order to cleanly support earlier datasets on the staging server. This + can be removed at some point, but it's not critical. + + Args: + dataset: Dataset object + + Returns: + A lowercase benchmark identifer string, including the value defined + by Metadata.SERVER_BENCHMARK_UNKNOWN if we can't find a value. + """ + benchmark = Metadata.getvalue(dataset, Metadata.SERVER_BENCHMARK) + + if not benchmark: + benchmark = Metadata.getvalue(dataset, "dataset.metalog.pbench.script") + if not benchmark: + benchmark = Metadata.SERVER_BENCHMARK_UNKNOWN + return benchmark + def __init__(self, config: PbenchServerConfig): super().__init__( config, @@ -70,17 +99,8 @@ def _get( datasets = params.query.get("datasets") benchmark_choice = None for dataset in datasets: - benchmark = Metadata.getvalue(dataset, Metadata.SERVER_BENCHMARK) - # Validate if all the selected datasets is of same benchmark - if not benchmark_choice: - benchmark_choice = benchmark - elif benchmark != benchmark_choice: - raise APIAbort( - HTTPStatus.BAD_REQUEST, - f"Selected dataset benchmarks must match: {benchmark_choice} and {benchmark} cannot be compared.", - ) - # Validate if the user is authorized to access the selected datasets + # Check that the user is authorized to read each dataset self._check_authorization( ApiAuthorization( ApiAuthorizationType.USER_ACCESS, @@ -90,6 +110,16 @@ def _get( ) ) + # Determine the dataset benchmark and check consistency + benchmark = DatasetsCompare.get_benchmark_name(dataset) + if not benchmark_choice: + benchmark_choice = benchmark + elif benchmark != benchmark_choice: + raise APIAbort( + HTTPStatus.BAD_REQUEST, + f"Selected dataset benchmarks must match: {benchmark_choice} and {benchmark} cannot be compared.", + ) + benchmark_type = BenchmarkName.__members__.get(benchmark.upper()) if not benchmark_type: raise APIAbort( diff --git a/lib/pbench/server/api/resources/datasets_visualize.py b/lib/pbench/server/api/resources/datasets_visualize.py index e0138f9e37..3aa29e945c 100644 --- a/lib/pbench/server/api/resources/datasets_visualize.py +++ b/lib/pbench/server/api/resources/datasets_visualize.py @@ -19,8 +19,8 @@ ParamType, Schema, ) +from pbench.server.api.resources.datasets_compare import DatasetsCompare from pbench.server.cache_manager import CacheManager -from pbench.server.database.models.datasets import Metadata class DatasetsVisualize(ApiBase): @@ -59,8 +59,7 @@ def _get( """ dataset = params.uri["dataset"] - - benchmark = Metadata.getvalue(dataset, Metadata.SERVER_BENCHMARK) + benchmark = DatasetsCompare.get_benchmark_name(dataset) benchmark_type = BenchmarkName.__members__.get(benchmark.upper()) if not benchmark_type: raise APIAbort( diff --git a/lib/pbench/test/unit/server/test_datasets_visualize.py b/lib/pbench/test/unit/server/test_datasets_visualize.py index 43658ff6cd..5547010360 100644 --- a/lib/pbench/test/unit/server/test_datasets_visualize.py +++ b/lib/pbench/test/unit/server/test_datasets_visualize.py @@ -132,3 +132,39 @@ def mock_get_metadata(_d: Dataset, _k: str, _u: Optional[User] = None) -> JSON: response = query_get_as("uperf_1", "test", HTTPStatus.BAD_REQUEST) assert response.json["message"] == "Unsupported Benchmark: hammerDB" assert extract_not_called + + @pytest.mark.parametrize( + "script,result", (("hammerDB", "hammerDB"), (None, "unknown")) + ) + def test_benchmark_fallback(self, query_get_as, monkeypatch, script, result): + """Test benchmark metadata fallback. + + If server.benchmark isn't defined, try dataset.metalog.pbench.script, + then fall back to "unknown". + + NOTE: This is deliberately not merged with test_unsupported_benchmark, + which it closely resembles, because the benchmark identification + tested here is intended to be a transitional workaround. + """ + + extract_not_called = True + + def mock_extract_data(*args, **kwargs) -> JSON: + nonlocal extract_not_called + extract_not_called = False + + @staticmethod + def mock_get_metadata(_d: Dataset, k: str, _u: Optional[User] = None) -> JSON: + if k == Metadata.SERVER_BENCHMARK: + return None + elif k == "dataset.metalog.pbench.script": + return script + else: + return "Not what you expected?" + + monkeypatch.setattr(CacheManager, "get_inventory", self.mock_get_inventory) + monkeypatch.setattr(Metadata, "getvalue", mock_get_metadata) + monkeypatch.setattr(QuisbyProcessing, "extract_data", mock_extract_data) + response = query_get_as("uperf_1", "test", HTTPStatus.BAD_REQUEST) + assert response.json["message"] == f"Unsupported Benchmark: {result}" + assert extract_not_called