From 629ea1e1e9d0f83cc8d9aa37907248b5c67eed3b Mon Sep 17 00:00:00 2001 From: Dan Wolfson Date: Mon, 12 Oct 2026 20:43:29 -0500 Subject: [PATCH] fix(overview_metrics): opt-in strict counts return None when not measured (ISSUE-123) count_elements, count_elements_by_property, counts_by_type, people_counts and usage_context_counts take strict=False by default (unchanged: a failed query counts as 0). With strict=True they return None when the fallback query failed or answered unexpectedly, or when it hit DEFAULT_CAP (a lower bound). A native count and a real zero are still numbers. strict is declared on the five analytic-registry entries so report specs can pass it. Signed-off-by: Dan Wolfson --- PYEGERIA_ISSUES.md | 93 +++++++++++-------- pyegeria/view/analytic_registry.py | 13 ++- pyegeria/view/overview_metrics.py | 92 +++++++++++++----- .../test_issue_123_strict_counts.py | 93 +++++++++++++++++++ 4 files changed, 225 insertions(+), 66 deletions(-) create mode 100644 tests/micro-tests/test_issue_123_strict_counts.py diff --git a/PYEGERIA_ISSUES.md b/PYEGERIA_ISSUES.md index 6db51209..6b877df6 100644 --- a/PYEGERIA_ISSUES.md +++ b/PYEGERIA_ISSUES.md @@ -148,7 +148,7 @@ content was changed. The numbered section list above describes the # Open Issues -**At a glance** — 6 open pyegeria items, 7 awaiting re-test, 6 open Egeria Server issues. +**At a glance** — 5 open pyegeria items, 7 awaiting re-test, 6 open Egeria Server issues. **Open pyegeria items** - ISSUE-140: The executor charts any dict with two or more top-level numbers, regardless of the result's state @@ -156,7 +156,6 @@ content was changed. The numbered section list above describes the - ISSUE-138: Analytic params are JSON-only, and only parameters named `mgr`/`ce`/`expert`/`client` are bound as clients, so a routine taking a reader object can't be named by its own dotted path - ISSUE-132: `DeleteElementRequestBody` serializes the cascade flag as `cascadeDelete`; Egeria's field is `cascadedDelete` and unknown fields are ignored, so no pyegeria cascade delete has ever cascaded - ISSUE-127: `CollectionManager.delete_collection(cascade=True)` silently never sends `cascadeDelete` -- same shape as ISSUE-62 -- ISSUE-123: `overview_metrics.count_elements` (and `_find`/`_element_count`) return `0`/`[]` on any failure — a failed measurement is indistinguishable from a real zero **Awaiting re-test** - ISSUE-125: `deleteEnumDef` answers 500 "unknown TypeDef" for an enum the server lists -- an enum added through the API cannot be removed @@ -303,43 +302,6 @@ Agreement. --- -### ISSUE-123: `overview_metrics.count_elements` (and `_find`/`_element_count`) return `0`/`[]` on any failure — a failed measurement is indistinguishable from a real zero - -**Layer:** Pyegeria · **Status:** open · **Found:** 2026-10-04, while -making the portal's Egeria Overview stop showing confident numbers during -an outage · **GitHub:** odpi/egeria-python#424 - -**Root cause, from reading `pyegeria/view/overview_metrics.py` (6.1.28):** - -- `_find(mgr, body, page_size)` wraps `find_metadata_elements` in a bare - `except Exception` and returns `[]`, logging only at `debug`. -- `_element_count(mgr, body, as_of)` tries the native count; if it is - unsupported *or raises*, it falls back to `len(_find(...))` — so a failed - query becomes `0`. -- `count_elements(...)`'s docstring states "0 on any failure (never - raises)". The callers `people_counts`, `usage_context_counts` and - `counts_by_type` inherit the same behaviour. - -**Effect:** a dashboard cannot tell "there are 0 elements" from "Egeria -timed out / returned 401 / 500". An outage or auth failure renders as a -confident `0`. A fallback count that returns exactly the page size is also -only a lower bound, not a total, and is not flagged. - -**Proposed fix:** return `None` on failure (or add a strict variant, e.g. -`count_elements(..., strict=True)` / `try_count_elements`, that raises or -returns `None`), keeping today's behaviour behind an explicit opt-in if -existing callers rely on it. Apply the same to `_find`/`_element_count` and -propagate `None` through `people_counts`, `usage_context_counts` and -`counts_by_type`. Log failures above `debug`. Flag a capped fallback count -as a lower bound. - -**Workaround in egeria-workspaces:** the portal's Overview handler calls -`find_metadata_elements` directly and returns `None` for failed or capped -queries, rendering "Not measured" instead of `0` (odpi/egeria-workspaces -#601, #607). - ---- - ### ISSUE-125: `deleteEnumDef` answers 500 "unknown TypeDef" for an enum the server lists -- an enum added through the API cannot be removed **Layer:** Egeria Server (not pyegeria) · **Status:** open; reported to the Egeria team (response 2026-10-05); Egeria fix merged to Egeria `main` 2026-10-06 (odpi/egeria#9363), not yet re-tested on a platform built from it · **Found:** @@ -1739,6 +1701,59 @@ pm.delete_collection(guid, cascade=True) --- +### ISSUE-123: `overview_metrics.count_elements` (and `_find`/`_element_count`) return `0`/`[]` on any failure — a failed measurement is indistinguishable from a real zero + +**Layer:** Pyegeria · **Status:** fixed 2026-10-12 (opt-in `strict=True`) · **Found:** 2026-10-04, while +making the portal's Egeria Overview stop showing confident numbers during +an outage · **GitHub:** odpi/egeria-python#424 + +**Root cause, from reading `pyegeria/view/overview_metrics.py` (6.1.28):** + +- `_find(mgr, body, page_size)` wraps `find_metadata_elements` in a bare + `except Exception` and returns `[]`, logging only at `debug`. +- `_element_count(mgr, body, as_of)` tries the native count; if it is + unsupported *or raises*, it falls back to `len(_find(...))` — so a failed + query becomes `0`. +- `count_elements(...)`'s docstring states "0 on any failure (never + raises)". The callers `people_counts`, `usage_context_counts` and + `counts_by_type` inherit the same behaviour. + +**Effect:** a dashboard cannot tell "there are 0 elements" from "Egeria +timed out / returned 401 / 500". An outage or auth failure renders as a +confident `0`. A fallback count that returns exactly the page size is also +only a lower bound, not a total, and is not flagged. + +**Proposed fix:** return `None` on failure (or add a strict variant, e.g. +`count_elements(..., strict=True)` / `try_count_elements`, that raises or +returns `None`), keeping today's behaviour behind an explicit opt-in if +existing callers rely on it. Apply the same to `_find`/`_element_count` and +propagate `None` through `people_counts`, `usage_context_counts` and +`counts_by_type`. Log failures above `debug`. Flag a capped fallback count +as a lower bound. + +**Workaround in egeria-workspaces:** the portal's Overview handler calls +`find_metadata_elements` directly and returns `None` for failed or capped +queries, rendering "Not measured" instead of `0` (odpi/egeria-workspaces +#601, #607). + +**Fixed 2026-10-12 — opt-in strict variant (owner's choice):** the default +is unchanged, so existing callers still get `0`. Pass `strict=True` to +`count_elements`, `count_elements_by_property`, `counts_by_type`, +`people_counts` or `usage_context_counts` to get `None` whenever the count is +not known: +- the fallback `find_metadata_elements` query raised, or answered with + something that is neither a list nor pyegeria's miss string; +- the fallback returned a full page (`DEFAULT_CAP`), which is only a lower + bound. + +A native count is trusted as is, even above the cap. A real zero (an empty +list, or the "No elements found" string) is still `0`. Each `None` logs a +warning saying why. `strict` is also declared on those five analytic-registry +entries, so a report spec can pass it. Test: +`tests/micro-tests/test_issue_123_strict_counts.py`. + +--- + ### ISSUE-122: `AssetMaker.get_catalog_targets` / `get_catalog_target` send `metadataElementTypeName="CatalogTarget"` (a relationship type) — server rejects with OMAG-COMMON-400-019, surfaced as SERVER_ERROR_500 **Status: FIXED and released.** Merged in #421 (`f2532bbd`, 2026-10-04) and shipped in **pyegeria 6.1.28** (and 6.1.29); present in the portal image's installed copy. Re-verified read-only on 2026-10-05 against the rebuilt platform: `get_catalog_targets` on the JDBC cataloguer returns "No elements found" in JSON, DICT and MD with no server error (it always answered `SERVER_ERROR_500` before the fix). The non-empty case was verified live on 2026-10-04, including the second defect found then (a blank DICT/MD record for the single get) and the `CatalogTargetProperties` field additions. diff --git a/pyegeria/view/analytic_registry.py b/pyegeria/view/analytic_registry.py index fbd54e3e..b40cb58f 100644 --- a/pyegeria/view/analytic_registry.py +++ b/pyegeria/view/analytic_registry.py @@ -140,6 +140,12 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None return AnalyticParam(name="as_of", type="str", description=description) +def _strict() -> AnalyticParam: + return AnalyticParam(name="strict", type="bool", default=False, + description="Return None instead of 0 when a count could not be measured " + "(query failed, or the fallback hit the page-size cap). ISSUE-123.") + + # ── Built-ins -- every analytic function already proven live via the Overview # dashboard's /api/overview/* endpoints (overview_handler.py), just not yet # reachable from a Dr.Egeria report spec. ────────────────────────────────── @@ -154,6 +160,7 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None AnalyticParam(name="type_name", type="str", description="Open metadata type to count, e.g. GlossaryTerm, DigitalProduct. Omit for any type."), _as_of(), + _strict(), AnalyticParam(name="classifications", type="list[str]", description="Match ANY of these classification names."), ], @@ -169,6 +176,7 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None AnalyticParam(name="type_map", type="list[tuple[str, str]]", required=True, description="(label, type_name) pairs to count, e.g. [('Data Stores', 'DataStore'), ...]."), _as_of(), + _strict(), ], ), "governed_coverage": AnalyticFunctionSpec( @@ -225,7 +233,7 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None returns="dict (persons, teams, organizations, itProfiles, communities)", generic=False, binding_note="Fixed to Person/Team/Organization/ITProfile/Community -- not a parameter.", - params=[_as_of()], + params=[_as_of(), _strict()], ), "feedback_summary": AnalyticFunctionSpec( name="feedback_summary", @@ -246,7 +254,7 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None returns="dict (informationSupplyChains, blueprints)", generic=False, binding_note="Fixed to InformationSupplyChain and SolutionBlueprint -- not a parameter.", - params=[_as_of()], + params=[_as_of(), _strict()], ), "semantic_grounding": AnalyticFunctionSpec( name="semantic_grounding", @@ -371,6 +379,7 @@ def _as_of(description: str = "ISO-8601 asOfTime for a point-in-time value (None AnalyticParam(name="property_value", type="str", required=True, description="Value to match the property against, e.g. ACTIVE."), _as_of(), + _strict(), ], ), "contextualised_coverage": AnalyticFunctionSpec( diff --git a/pyegeria/view/overview_metrics.py b/pyegeria/view/overview_metrics.py index 16bf9155..a06bf8de 100644 --- a/pyegeria/view/overview_metrics.py +++ b/pyegeria/view/overview_metrics.py @@ -163,6 +163,13 @@ def _json_list(raw) -> list: def _find(mgr, body: dict, page_size: int = DEFAULT_CAP) -> list: """Run a FindRequestBody, always returning a (possibly empty) list. Never raises.""" + return _find_or_none(mgr, body, page_size) or [] + + +def _find_or_none(mgr, body: dict, page_size: int = DEFAULT_CAP) -> Optional[list]: + """Run a FindRequestBody: a list of elements, [] when nothing matched (a list + or pyegeria's "No elements found" string), or None when the query failed or + answered with something else. Never raises.""" try: # find_metadata_elements sends body exactly as given (no injected # defaults) -- graphQueryDepth/startFrom/pageSize all belong in the @@ -173,15 +180,26 @@ def _find(mgr, body: dict, page_size: int = DEFAULT_CAP) -> list: # them as parameters at all, to avoid this drifting stale again). b = {**body, "graphQueryDepth": 0, "startFrom": 0, "pageSize": page_size} raw = mgr.find_metadata_elements(b) - return [el for el in (raw if isinstance(raw, list) else []) if isinstance(el, dict)] except Exception as exc: # noqa: BLE001 -- best-effort, degrade don't fail logger.debug(f"overview_metrics _find failed for {body.get('metadataElementTypeName')!r}: {exc}") - return [] + return None + if isinstance(raw, list): + return [el for el in raw if isinstance(el, dict)] + if isinstance(raw, str): + return [] # pyegeria's miss sentinel + logger.debug(f"overview_metrics _find: unexpected result for {body.get('metadataElementTypeName')!r}: " + f"{type(raw).__name__}") + return None -def _element_count(mgr, body: dict, as_of: Optional[str] = None) -> int: +def _element_count(mgr, body: dict, as_of: Optional[str] = None, strict: bool = False) -> Optional[int]: """Count elements matching a FindRequestBody -- native when available, else - len(find_metadata_elements(...)).""" + len(find_metadata_elements(...)). + + strict=False: 0 when the fallback query fails (the historical behaviour). + strict=True: None when the count is not known -- the fallback query failed, + or it returned a full page (DEFAULT_CAP), which is only a lower bound. + """ b = dict(body) if as_of: b["asOfTime"] = as_of @@ -193,7 +211,18 @@ def _element_count(mgr, body: dict, as_of: Optional[str] = None) -> int: return c except Exception as exc: # noqa: BLE001 -- old server: mark unsupported, fall back _disable_native(mgr, exc) - return len(_find(mgr, b)) + if not strict: + return len(_find(mgr, b)) + found = _find_or_none(mgr, b) + if found is None: + logger.warning(f"overview_metrics: count of {body.get('metadataElementTypeName') or 'elements'!r} " + f"not measured -- the query failed") + return None + if len(found) >= DEFAULT_CAP: + logger.warning(f"overview_metrics: count of {body.get('metadataElementTypeName') or 'elements'!r} " + f"not measured -- the fallback hit the {DEFAULT_CAP} cap, so {len(found)} is only a lower bound") + return None + return len(found) def count_elements( @@ -201,12 +230,17 @@ def count_elements( type_name: Optional[str] = None, as_of: Optional[str] = None, classifications: Optional[Sequence[str]] = None, -) -> int: + strict: bool = False, +) -> Optional[int]: """ Count ACTIVE elements, optionally of a given type and/or matching ANY of the given classifications. Uses native `count_metadata_elements` when the client + server support it, else `len(find_metadata_elements(...))`. + By default a failed query counts as 0, so an outage looks like an empty + catalog. Pass strict=True to get None instead whenever the number is not + known (ISSUE-123). + Parameters ---------- mgr : MetadataExpert (or compatible client) @@ -214,10 +248,12 @@ def count_elements( as_of : ISO-8601 asOfTime for a point-in-time count (None = now) classifications : classification names to match ANY of -- keep to a single call's worth (some connectors mishandle 2+ ANY conditions) + strict : when True, return None if the query failed, or if the fallback + count hit DEFAULT_CAP (a lower bound, not a total) Returns ------- - int -- 0 on any failure (never raises) + int -- 0 on any failure (never raises); with strict=True, Optional[int] """ body: Dict[str, Any] = {"class": "FindRequestBody", "limitResultsByStatus": ["ACTIVE"]} if type_name: @@ -227,7 +263,7 @@ def count_elements( "class": "SearchClassifications", "matchCriteria": "ANY", "conditions": [{"name": n} for n in classifications], } - return _element_count(mgr, body, as_of) + return _element_count(mgr, body, as_of, strict) def count_elements_by_property( @@ -236,7 +272,8 @@ def count_elements_by_property( property_name: str, property_value: str, as_of: Optional[str] = None, -) -> int: + strict: bool = False, +) -> Optional[int]: """ Count ACTIVE elements of a given type whose named string property equals a given value -- e.g. counting DigitalProduct elements by deploymentStatus @@ -253,10 +290,11 @@ def count_elements_by_property( property_name : the element property to match (e.g. "deploymentStatus") property_value : the exact string value to match as_of : ISO-8601 asOfTime for a point-in-time count (None = now) + strict : as for count_elements -- None instead of 0 when not measured Returns ------- - int -- 0 on any failure (never raises) + int -- 0 on any failure (never raises); with strict=True, Optional[int] """ body: Dict[str, Any] = { "class": "FindRequestBody", @@ -272,7 +310,7 @@ def count_elements_by_property( }], }, } - return _element_count(mgr, body, as_of) + return _element_count(mgr, body, as_of, strict) def count_relationships(ce, relationship_type: str, as_of: Optional[str] = None, expert=None) -> Optional[int]: @@ -322,16 +360,18 @@ def count_relationships(ce, relationship_type: str, as_of: Optional[str] = None, return None -def counts_by_type(mgr, type_map: Sequence[Tuple[str, str]], as_of: Optional[str] = None) -> List[Dict[str, Any]]: +def counts_by_type(mgr, type_map: Sequence[Tuple[str, str]], as_of: Optional[str] = None, + strict: bool = False) -> List[Dict[str, Any]]: """ Per-type counts for a set of (label, type_name) pairs -- e.g. the assets- by-type composition breakdown. Unknown/wrong types just yield 0 rather - than failing the whole call. + than failing the whole call. With strict=True, a count that could not be + measured is None (see count_elements). Returns a list of {"label": ..., "type": ..., "count": ...} dicts, in the order given. """ - return [{"label": label, "type": type_name, "count": count_elements(mgr, type_name, as_of)} + return [{"label": label, "type": type_name, "count": count_elements(mgr, type_name, as_of, strict=strict)} for label, type_name in type_map] @@ -1031,7 +1071,7 @@ def _update_time(el: dict): return None -def people_counts(mgr, as_of: Optional[str] = None) -> Dict[str, int]: +def people_counts(mgr, as_of: Optional[str] = None, strict: bool = False) -> Dict[str, Optional[int]]: """ Person / Team / Organization / ITProfile / Community counts via native element counts (sub-second where an older find-and-bucket approach would @@ -1041,14 +1081,15 @@ def people_counts(mgr, as_of: Optional[str] = None) -> Dict[str, int]: raw count is the authoritative repository count. Returns {"persons": int, "teams": int, "organizations": int, - "itProfiles": int, "communities": int}. + "itProfiles": int, "communities": int}. With strict=True, a count that + could not be measured is None (see count_elements). """ return { - "persons": count_elements(mgr, "Person", as_of), - "teams": count_elements(mgr, "Team", as_of), - "organizations": count_elements(mgr, "Organization", as_of), - "itProfiles": count_elements(mgr, "ITProfile", as_of), - "communities": count_elements(mgr, "Community", as_of), + "persons": count_elements(mgr, "Person", as_of, strict=strict), + "teams": count_elements(mgr, "Team", as_of, strict=strict), + "organizations": count_elements(mgr, "Organization", as_of, strict=strict), + "itProfiles": count_elements(mgr, "ITProfile", as_of, strict=strict), + "communities": count_elements(mgr, "Community", as_of, strict=strict), } @@ -1073,17 +1114,18 @@ def feedback_summary(ce, as_of: Optional[str] = None) -> Dict[str, Any]: return {"byType": by_type, "total": sum(by_type.values())} -def usage_context_counts(mgr, as_of: Optional[str] = None) -> Dict[str, int]: +def usage_context_counts(mgr, as_of: Optional[str] = None, strict: bool = False) -> Dict[str, Optional[int]]: """ InformationSupplyChain and SolutionBlueprint counts -- what puts assets in a *usage context* ("this store feeds the Clinical Trial supply chain", "this component realises the Sales blueprint"). - Returns {"informationSupplyChains": int, "blueprints": int}. + Returns {"informationSupplyChains": int, "blueprints": int}. With + strict=True, a count that could not be measured is None (see count_elements). """ return { - "informationSupplyChains": count_elements(mgr, "InformationSupplyChain", as_of), - "blueprints": count_elements(mgr, "SolutionBlueprint", as_of), + "informationSupplyChains": count_elements(mgr, "InformationSupplyChain", as_of, strict=strict), + "blueprints": count_elements(mgr, "SolutionBlueprint", as_of, strict=strict), } diff --git a/tests/micro-tests/test_issue_123_strict_counts.py b/tests/micro-tests/test_issue_123_strict_counts.py new file mode 100644 index 00000000..76c5b912 --- /dev/null +++ b/tests/micro-tests/test_issue_123_strict_counts.py @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Contributors to the ODPi Egeria project. +""" +ISSUE-123: count_elements turns a failed query into 0, so an outage looks like +an empty catalog. strict=True returns None whenever the count is not known; +the default keeps the old behaviour for existing callers. +""" +from unittest.mock import MagicMock + +import pytest + +from pyegeria.view import analytic_registry +from pyegeria.view import overview_metrics as om + + +def _mgr(native=None, find_result=None, find_error=None): + attrs = ["platform_url", "view_server", "find_metadata_elements"] + if native is not None: + attrs.append("count_metadata_elements") + m = MagicMock(spec=attrs) + m.platform_url, m.view_server = "https://localhost:9443", "test-server" + if native is not None: + m.count_metadata_elements = native + if find_error is not None: + m.find_metadata_elements.side_effect = find_error + else: + m.find_metadata_elements.return_value = find_result + return m + + +@pytest.fixture(autouse=True) +def _reset_seam_caches(): + om._count_caps.clear() + om._native_server_ok.clear() + yield + om._count_caps.clear() + om._native_server_ok.clear() + + +def test_default_still_returns_zero_on_failure(): + assert om.count_elements(_mgr(find_error=RuntimeError("401")), "Asset") == 0 + + +def test_strict_returns_none_when_the_query_fails(): + assert om.count_elements(_mgr(find_error=RuntimeError("401")), "Asset", strict=True) is None + + +def test_strict_returns_none_when_native_and_fallback_both_fail(): + mgr = _mgr(native=MagicMock(side_effect=RuntimeError("timeout")), find_error=RuntimeError("timeout")) + assert om.count_elements(mgr, "Asset", strict=True) is None + + +def test_strict_real_zero_is_zero(): + # pyegeria answers a miss with its sentinel string, not an empty list. + assert om.count_elements(_mgr(find_result="No elements found"), "Asset", strict=True) == 0 + assert om.count_elements(_mgr(find_result=[]), "Asset", strict=True) == 0 + + +def test_strict_returns_none_for_a_capped_fallback(): + full_page = [{"i": i} for i in range(om.DEFAULT_CAP)] + assert om.count_elements(_mgr(find_result=full_page), "Asset") == om.DEFAULT_CAP + assert om.count_elements(_mgr(find_result=full_page), "Asset", strict=True) is None + + +def test_strict_returns_none_for_an_unexpected_result(): + assert om.count_elements(_mgr(find_result={"weird": True}), "Asset", strict=True) is None + + +def test_strict_native_count_is_trusted_even_above_the_cap(): + mgr = _mgr(native=MagicMock(return_value={"count": om.DEFAULT_CAP + 5})) + assert om.count_elements(mgr, "Asset", strict=True) == om.DEFAULT_CAP + 5 + + +def test_strict_by_property(): + mgr = _mgr(find_error=RuntimeError("500")) + assert om.count_elements_by_property(mgr, "DigitalProduct", "deploymentStatus", "ACTIVE") == 0 + assert om.count_elements_by_property(mgr, "DigitalProduct", "deploymentStatus", "ACTIVE", + strict=True) is None + + +def test_strict_propagates_through_the_composite_counts(): + mgr = _mgr(find_error=RuntimeError("down")) + assert om.counts_by_type(mgr, [("Stores", "DataStore")], strict=True)[0]["count"] is None + assert set(om.people_counts(mgr, strict=True).values()) == {None} + assert set(om.usage_context_counts(mgr, strict=True).values()) == {None} + assert set(om.people_counts(mgr).values()) == {0} + + +@pytest.mark.parametrize("name", ["count_elements", "count_elements_by_property", "counts_by_type", + "people_counts", "usage_context_counts"]) +def test_strict_is_declared_for_report_specs(name): + spec = analytic_registry.get_analytic_registry()[name] + assert any(p.name == "strict" and p.type == "bool" for p in spec.params)