Skip to content

[Fix] th-cli --version Surfaces Backend And SDK Versions (#999)#85

Open
abhisheksingh-esp wants to merge 1 commit into
project-chip:v2.16-cli-developfrom
abhisheksingh-esp:fix/999_cli_version_command_backend_versions
Open

[Fix] th-cli --version Surfaces Backend And SDK Versions (#999)#85
abhisheksingh-esp wants to merge 1 commit into
project-chip:v2.16-cli-developfrom
abhisheksingh-esp:fix/999_cli_version_command_backend_versions

Conversation

@abhisheksingh-esp
Copy link
Copy Markdown

Summary

Fixes #999. th-cli --version now surfaces the backend and SDK versions returned by GET /api/v1/version.

Root cause

get_versions() in th_cli/utils.py was annotated -> dict but actually returned a Pydantic TestHarnessBackendVersion instance from the autogenerated SyncVersionApi. The caller in main.py::get_extended_help iterated it with .items(), which raised AttributeError on the model. The bare except Exception: swallowed it and replaced the block with "Not able to retrieve versions from server.", so the five backend fields (version, sha, sdk_sha, sdk_docker_tag, db_revision) were dropped CLI-side even though the API response was fine.

Fix

Convert the Pydantic v2 model to a dict via model_dump() in get_versions(). The existing .items() loop in get_extended_help then renders all backend fields without further changes.

Screenshot 2026-05-25 at 12 53 52 PM

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the get_versions function in th_cli/utils.py to return a dictionary representation of the version information using model_dump(). The reviewer suggested using model_dump(mode='json') to ensure that any future complex data types are correctly serialized for JSON output, which improves the robustness of the CLI utility.

Comment thread th_cli/utils.py
version_api = sync_apis.version_api
versions_info = version_api.get_test_harness_backend_version_api_v1_version_get()
return versions_info
return versions_info.model_dump()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider using model_dump(mode='json') instead of model_dump(). While the current fields in TestHarnessBackendVersion are all strings, using mode='json' ensures that any future complex types (like datetimes or enums) are automatically serialized to JSON-compatible formats. This makes the function more robust for its intended use in the CLI, where the results are likely to be printed or serialized.

Suggested change
return versions_info.model_dump()
return versions_info.model_dump(mode='json')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant