fix(pyegeria): rebuild the OMVS endpoint audit tool + fix what it surfaces - #277
Merged
dwolfson merged 4 commits intoAug 18, 2026
Conversation
…ld the audit tool
The OMVS audit script could not detect the defect classes it existed to find,
so a batch of endpoint bugs was reported as "covered". Rebuild the tool, then
fix what it actually surfaces.
Tooling (scripts/omvs_audit.py):
- Verb mapping was wrong: it hardcoded the _async_get_*_request helpers to GET.
Every helper in _server_client.py is POST. Verbs are now discovered by
parsing that file, and a direct _async_make_request wins over a helper
(some methods call a GUID-resolution helper first).
- URL extraction was regex-based and yielded empty paths for multi-line
f-strings, making whole modules look clean. Now AST-based.
- Path comparison stripped the leading service segment and trailing verbs
(/attach vs /detach), masking exactly the bugs worth finding. Now compares
full normalised paths.
- Adds request-body class comparison, structural URL lint, --service/--quiet/
--http-dir, a loud failure when the gitignored ground-truth dir is absent,
and exit 1 on defect so it can gate CI.
Endpoint fixes:
- platform_services: drop the duplicate _async_get_platform_origin /
get_platform_origin pair. They shadowed BasePlatformClient's working
implementation and omitted is_json=False, so every call raised
PyegeriaInvalidParameterException parsing a plain-text response as JSON.
- data_discovery: six annotation getters issued GET with query params where
Egeria expects POST with a body. Routed through _async_get_name_request
(FilterRequestBody) and _async_get_results_body_request (ResultsRequestBody),
and corrected three paths: by-type -> by-annotation-type, a spurious path
parameter on by-analysis-step, and extensions -> annotation-extensions.
- metadata_expert: match-criteria and property-comparison-operators were
missing their /metadata-search/ prefix and the -values suffix.
- asset_maker, external_links: restore get_actions_for_requester and
get_external_identifiers_by_guid. The renames diverged from Egeria's own
spelling (getActionsForRequester, getExternalIdentifiersByGUID) and broke
test_asset_maker.py and test_external_links.py.
- server_operations: ops_command_root carried a trailing slash, so four of
five call sites built //servers/. Normalised the root instead.
- actor_manager: it_profiles -> it-profiles on detach (attach was correct).
- collection_manager: missing / before {agreement_guid} on agreement-item
detach produced /agreements<guid>/.
Audit now reports 557 OK / 102 mismatch / 0 lint, down from 344 mismatches
(mostly false positives) under the old script. Remaining mismatches
concentrate in connection-maker (34), governance-officer (18) and
metadata-expert (15) and are untriaged.
Verified: micro-tests pass, functional tests collect, the previously broken
call sites resolve. The tool is regression-checked against a reconstructed
pre-fix method to confirm it flags the original defect.
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…ribute name
resolve_roots() only recognised attributes whose name ended in command_root,
command_base, base_path or command_url. connection_maker.py assigns its root
to self.base_url, so nothing resolved it: every URL flattened to "/{}/..."
and all 34 of its endpoints were reported as path mismatches.
Detect roots by value instead -- any self.<attr> assigned a string containing
"/open-metadata/" is a service root -- keeping the name heuristic as a
fallback. This is self-maintaining as new clients are added.
Verified by set-comparing connection-maker's 34 endpoints against
Egeria-api-connection-maker.http independently of the name-matching logic:
34/34 exact, no difference in either direction. The module was already
correct; the finding was entirely a tool artifact.
Audit now reports 598 OK / 61 mismatch (was 557 / 102).
Not changed: connection_maker, privacy_officer and security_officer build
URLs from self.server_name while 31 other clients use self.view_server. Since
the OMVS clients set view_server = server_name, and server_name itself
defaults to the configured view server, both forms produce identical URLs --
a cosmetic inconsistency, not a defect.
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…e body as a set
governance-officer reported 18 mismatches; 17 were tool artifacts.
Tooling (scripts/omvs_audit.py):
- Resolve bare service markers. governance_officer.py interpolates
self.url_marker = "governance-officer" straight into its URLs. Only full-URL
roots were resolved, so every path flattened to "/{}/..." and 16 endpoints
reported a bogus mismatch. Now resolves constant string attributes whose name
looks like a URL marker and whose value looks like a path segment, so
identity/credential attributes are never substituted into a path.
- Derive the request-body class from the helper actually called, not from the
first *RequestBody identifier in the method. The old heuristic reported
"FilterRequestBody" for a method that in fact sends GetRequestBody.
- Compare bodies as a set, not a single name. Several helpers accept a union
(e.g. dict | UpdateElementRequestBody | UpdateClassificationRequestBody);
picking one name out of a union invented two more false mismatches.
Endpoint fix:
- governance_officer.detach_design_from_implementation annotated its body as
DeleteElementRequestBody while calling _async_delete_relationship_request,
whose validator accepts only DeleteRelationshipRequestBody or dict. A caller
following the annotation and passing a DeleteElementRequestBody object hits
the validator's else branch, which returns None -- so the guarded call is
skipped and the detach silently never happens. Corrected to
DeleteRelationshipRequestBody (matches the .http ground truth).
Audit now reports 603 OK / 56 mismatch (was 598 / 45 -- the count rose because
accurate body detection surfaced real findings the old heuristic masked).
governance-officer has one finding left, deliberately not changed:
getGovernanceActionProcessGraph sends GetRequestBody where the .http documents
ResultsRequestBody. GetRequestBody is a strict subset -- it lacks the paging,
sequencing and anchor fields -- so switching helpers is a behaviour change that
needs verification against a live server first.
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…ndpoint-audit-omvs-audit-followup
dwolfson
deleted the
fix/pyegeria-http-endpoint-audit-omvs-audit-followup
branch
August 18, 2026 16:23
dwolfson
added a commit
that referenced
this pull request
Aug 18, 2026
…ody, arity, classification helper These three fixes were made and verified in an isolated Agent worktree (egeria-python-omvs-audit-followup) that the harness auto-cleaned once its commits were pushed and PR #277 merged. Because these three fixes were still uncommitted at that point, they were lost with the directory rather than merged. Reapplied here from scratch on top of PR #277's tip (confirmed absent via egeria-python-73, a peer session, before redoing). 1. governance_officer.get_governance_action_process_graph sent GetRequestBody where Egeria-api-governance-officer.http documents ResultsRequestBody. Added an opt-in body_model param to _async_get_guid_request (default unchanged: GetRequestBody) so this one call can send the documented body without touching the other 24 callers of that helper. Kept on _async_get_guid_request rather than switching to _async_get_results_body_request, because only this helper reads the singular "elementGraph" response key the graph endpoint returns. 2. Six *_from_template methods (actor_manager x3, data_discovery, subject_area, time_keeper) called _async_create_element_body_request instead of _async_create_element_from_template -- silently dropping the 8 fields only TemplateRequestBody has (template_guid, placeholder_property_values, replacement_properties, deep_copy, ...) via PyegeriaModel's extra='ignore'. The element was created with no template ever applied, no error raised. Also fixed 4 call sites (actor_manager x3, location_arena) passing an extra "POST" positional argument to _async_create_element_from_template, which only takes (url, body) -- confirmed this raises TypeError before any request is sent. 3. classification_explorer's clear_known_duplicate_classification and clear_consolidated_duplicate_classification annotated DeleteClassificationRequestBody but called _async_delete_relationship_request, whose validator only accepts DeleteRelationshipRequestBody or dict -- passing the annotated type hits the validator's else branch, which returns None, so the guarded call is skipped and the clear silently never happens. Switched both to the dedicated _async_delete_classification_request helper (the other 12 clear_* methods in this file already used it correctly). Verified: full pyegeria/ compiles, all touched modules import, zero _async_create_element_from_template call sites now exceed its 2-param arity, micro-tests pass, functional tests collect. Audit: 612 OK / 47 mismatch (was 605/54 on this base) -- governance-officer and the six from-template methods now report 0 mismatches; classification-explorer down to the 6 unrelated findings that predate this pass. Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The OMVS audit script (
scripts/omvs_audit.py) could not detect the defect classes it existed to find, so a batch of real endpoint bugs was reported as "covered." Rebuilds the tool, then fixes what it actually surfaces once working correctly:Tooling fixes:
_async_get_*_requesthelpers to GET. Every helper in_server_client.pyis POST. Verbs are now discovered by parsing that file, and a direct_async_make_requestcall wins over a helper (some methods call a GUID-resolution helper first)./attachvs/detach), masking exactly the bugs worth finding. Now compares full normalised paths.resolve_roots()only recognised attributes ending incommand_root/command_base/base_path/command_urlby name.connection_maker.pyassigns its root toself.base_url, so nothing resolved it — every URL flattened to"/{}/..."and all 34 of its endpoints were reported as path mismatches. Now resolves service roots by value, not attribute name.Real bug found and fixed once the tool actually worked: governance-officer audit — resolved
url_marker, compares body as a set (see commit for detail).Base commit for this branch was
fix/pyegeria-http-endpoint-audit(now merged as #275) — diff againstmainhere is scoped to just this branch's own 3 commits.🤖 Generated with Claude Code