-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
[ansible/venv] Use PyPi packages instead of GitHub #229
Conversation
WalkthroughThe pull request updates dependency management for the OVOS Installer by replacing direct GitHub repository references with specific package names across multiple requirement files. The changes focus on HiveMind-related dependencies, introducing more standardized package naming conventions with hyphens and adding several new OVOS and HiveMind packages. The modifications also include conditional dependencies based on system locale, CPU capabilities, and available I2C devices. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ansible/roles/ovos_installer/templates/virtualenv/server-requirements.txt.j2 (1)
2-4
: Consider pinning package versions for reproducible builds.While moving from GitHub to PyPi packages improves stability, consider pinning specific versions to ensure reproducible builds and avoid unexpected breaking changes.
-hivemind-bus-client -hivemind-core -hivemind-presence +hivemind-bus-client==<version> +hivemind-core==<version> +hivemind-presence==<version>ansible/roles/ovos_installer/templates/virtualenv/satellite-requirements.txt.j2 (1)
Line range hint
3-8
: Consider version pinning for core dependencies.While the package structure is good, consider pinning versions for reproducibility.
-ovos-audio[extras] -ovos-classifiers -ovos-dinkum-listener[extras,linux] -ovos-PHAL[linux] -ovos-phal-plugin-ipgeo -ovos-tts-plugin-polly +ovos-audio[extras]==<version> +ovos-classifiers==<version> +ovos-dinkum-listener[extras,linux]==<version> +ovos-PHAL[linux]==<version> +ovos-phal-plugin-ipgeo==<version> +ovos-tts-plugin-polly==<version>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ansible/roles/ovos_installer/templates/virtualenv/listener-requirements.txt.j2
(1 hunks)ansible/roles/ovos_installer/templates/virtualenv/satellite-requirements.txt.j2
(1 hunks)ansible/roles/ovos_installer/templates/virtualenv/server-requirements.txt.j2
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ansible/roles/ovos_installer/templates/virtualenv/listener-requirements.txt.j2
🔇 Additional comments (3)
ansible/roles/ovos_installer/templates/virtualenv/server-requirements.txt.j2 (1)
1-4
: LGTM! Successfully migrated from GitHub to PyPi packages.
The transition from GitHub URLs to PyPi packages for HiveMind dependencies is a good improvement that:
- Reduces build complexity
- Improves dependency resolution
- Makes version management easier
ansible/roles/ovos_installer/templates/virtualenv/satellite-requirements.txt.j2 (2)
1-2
: LGTM! HiveMind dependencies successfully migrated to PyPi.
The HiveMind dependencies have been properly converted from GitHub URLs to PyPi packages.
Line range hint 10-24
: LGTM! Well-structured conditional dependencies.
The conditional dependencies are well organized and properly handle:
- Locale-specific TTS plugins
- CPU capability checks
- Hardware-specific PHAL configurations
This structure ensures that only necessary dependencies are installed based on the system configuration.
Let's verify the Ansible variables used in conditions:
✅ Verification successful
All variables used in the requirements file are properly defined and used consistently across the role
The verification confirms that all conditional variables (ovos_installer_locale
, ovos_installer_cpu_is_capable
, and ovos_installer_i2c_devices
) are properly used throughout the role:
ovos_installer_locale
is used for language configuration in multiple files including mycroft.conf.j2 and docker composerovos_installer_cpu_is_capable
is used consistently for CPU-dependent features in venv.yml, mycroft.conf.j2, and telemetryovos_installer_i2c_devices
is properly handled as a JSON array for hardware detection in various service configurations
The conditional dependencies in satellite-requirements.txt.j2 align perfectly with the role's overall configuration patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the Ansible variables used in conditions are defined in the role
rg -A 2 "ovos_installer_(locale|cpu_is_capable|i2c_devices)" ansible/roles/ovos_installer/
Length of output: 6612
Summary by CodeRabbit
New Features
hivemind-bus-client
,hivemind-voice-sat
, andovos-audio[extras]
.Bug Fixes
Documentation