CMLDEV-1110: added reload_definitions method to NodeImageDefinitions;…

[BregaladTaran]

… Included brief test coverage;

[tmikuska]

[Bug (New) - Confidence: 90%] Missing @_requires_version("2.10.0") decorator.

The server endpoint is gated to CML 2.10+ via CMLVersion.V2_10.introduced() in webserver/simple_webserver/api/system.py. Without the decorator, calling this method against a 2.9.x controller produces a generic 404 / HTTPError instead of the codebase's standard FeatureNotSupported exception. Sibling version-gated methods in lab_repository.py (sync_lab_repositories, refresh_lab_repositories, get_lab_repositories, add_lab_repository) all use this pattern.

Suggested change:

from ..utils import _requires_version, get_url_from_template

# ...

@_requires_version("2.10.0")
def reload_definitions(self) -> dict[str, dict[str, list[str]]]:
    ...

[tmikuska]

[Nit (New) - Confidence: 50%] dict[str, Any] discards the well-defined server response shape.

The server returns DefinitionReportResponse — two fixed top-level keys (node_definitions, image_definitions), each mapping to a dict of unchanged/updated/new/removed/failed lists of strings. A narrower annotation conveys the same shape without introducing new public symbols:

def reload_definitions(self) -> dict[str, dict[str, list[str]]]:
    ...

Other methods on this class are equally loose (dict[str, Any] / list[dict[str, Any]]), so this is optional polish for consistency.

[tmikuska]

[Nit (New) - Confidence: 40%] Test hard-codes "reload_definitions" instead of going through _url_for("reload_defs") / _URL_TEMPLATES. Renaming the template key without updating the URL would not be caught here.

This matches the existing pattern in this file though (other tests don't assert on the URL at all), so optional.

[tmikuska]

AI-Assisted Code Review Summary

This review was performed by an LLM. All comments — especially line numbers, suggested code, and version/permission claims — should be verified by a human before acting on them.

Linked Jira: CMLDEV-1110

Scope check

Tight, focused PR (2 files, +63 / −0). Cross-checked against the server-side endpoint in the simple monorepo (webserver/simple_webserver/api/system.py:685-731): URL (reload_definitions), HTTP verb (PUT), and response shape (DefinitionReportResponse) all match. The example payload in the test faithfully reflects the server schema.

Two server-side constraints aren't reflected on the client:

• Endpoint is annotated CMLVersion.V2_10.introduced() — CML 2.10+ only.
• Endpoint is gated by IsAdminPermDepends — admin only.

Major (should fix)

• virl2_client/models/node_image_definition.py:317 — reload_definitions is missing @_requires_version("2.10.0"). Without it, older controllers raise a generic 404 instead of the standard FeatureNotSupported. (See inline comment.)

Minor (negligible / nits)

• virl2_client/models/node_image_definition.py:319 — Docstring should mention the CML >= 2.10 requirement and the admin-only access constraint. (See inline comment.)
• virl2_client/models/node_image_definition.py:317 — Return type dict[str, Any] discards the well-defined response shape; consider dict[str, dict[str, list[str]]] for slightly stronger typing without introducing new public symbols. Other methods on the class are equally loose, so this is optional. (See inline comment.)
• tests/test_node_image_definitions.py:323 — Test hard-codes the URL string instead of going through _url_for. Matches the existing pattern, optional. (See inline comment.)

Things done well

• Verb/path/response shape all match the authoritative server contract.
• Test follows the tests/AGENTS.md conventions (LLM-generated note, reST docstring, plain MagicMock, single behavior per test).
• Example response in the test matches the server's make_def_report_error output format for the failed entry, which is a nice touch.
• URL template centralized in _URL_TEMPLATES, consistent with the rest of the class.

---

Confidence percentages reflect certainty: lower values may be false positives. Origin column: New = introduced by this PR, Pre-existing = already in the codebase (consider fixing opportunistically). All findings here are New.