-
Notifications
You must be signed in to change notification settings - Fork 98
Improved 'netlab version' printout #2912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reports: * All modules/packages used by netlab core (the list is still updated manually) * All core ansible components Resolves #2910
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the netlab version command to provide comprehensive reporting of all dependencies used by netlab core and Ansible components. The implementation refactors the version checking logic into reusable functions and uses the modern importlib.metadata API for version detection.
Key changes:
- Adds tracking for previously unreported core dependencies ('requests', 'filelock', 'rich')
- Creates a dedicated section for Ansible components (ansible, ansible-core, ansible-pylibssh)
- Refactors version detection into two specialized functions:
module_version()for module-based checks andpackage_version()for metadata-based checks
| print(f' Cannot find {package} version: {str(ex)}') | ||
| except Exception as ex: | ||
| if package not in MISSING_OK: | ||
| print(f' {package}: not installed {str(ex)}') |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a package is in MISSING_OK and cannot be imported, no message is printed at all. This creates inconsistent output where MISSING_OK packages silently disappear from the version report. Consider printing a message like "not installed (optional)" for MISSING_OK packages to maintain consistency in the output.
| print(f' {package}: not installed {str(ex)}') | |
| print(f' {package}: not installed {str(ex)}') | |
| else: | |
| print(f' {package}: not installed (optional)') |
|
|
||
| def run(args: typing.List[str]) -> None: | ||
| global MODULES, MISSING_OK | ||
| global MODULES, PACKAGES, ANSIBLE_PACKAGES, MISSING_OK |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global declaration is unnecessary since the function only reads these variables without modifying them. Remove the global statement to improve code clarity.
| global MODULES, PACKAGES, ANSIBLE_PACKAGES, MISSING_OK |
While implementing a Copilog suggestion, I realized it looks better if we split required and optional packages in two lists
DanPartelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Thank you, Ivan.
Reports:
Resolves #2910