Update hardware-exporter templates - #552
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated ExecStart references {{ CHARMDIR }}/venv/python, which is unlikely to exist in a standard virtualenv and can prevent the service from starting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adjusts the generated systemd unit template for the hardware exporter so it runs under the charm’s virtualenv correctly after the move to the uv plugin venv layout (fixing the “No module named prometheus_hardware_exporter” regression noted in #551 / #530).
Changes:
- Removes the
PYTHONPATH={{ CHARMDIR }}/venvapproach from the systemd unit. - Updates
ExecStartto run the exporter with the virtualenv activated.
File summaries
| File | Description |
|---|---|
| src/templates/hardware-exporter.service.j2 | Updates systemd ExecStart strategy to ensure the exporter runs with the correct venv/site-packages layout. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
e8bccbb to
4240428
Compare
|
it's weird that this issue was not caught during the manual tests: #531 |
There was a problem hiding this comment.
🟢 Approval recommended
The change is small and directly addresses the reported regression; the remaining feedback is a robustness/maintainability improvement rather than a functional blocker.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
- charm plugin: venv/ ├── cachetools ... └── zipp-3.23.1.dist-info - uv plugin: venv/ ├── bin │ └── activate ├── lib │ └── python3.12 │ └── site-packages │ ├── _yaml ... │ └── yaml └── pyvenv.cfg
4240428 to
50ce1e3
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the reported regression by ensuring the service runs under the venv interpreter, and the template variables used are already supplied by the renderer.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Tested refresh behavior in |
uv plugin changed site-package layout from
to
which means the
Environment=PYTHONPATH={{ CHARMDIR }}/venvno longer import the site-package correctly. The safest way is to use thepythoncreated by the venv.Fixes: #551
Regression from: #530