Conversation
Minor: remove pointless imports
Minor: mark systemd based runner as experimental
eb56348 to
03b5dcb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2155 +/- ##
==========================================
- Coverage 53.34% 53.02% -0.32%
==========================================
Files 399 395 -4
Lines 42907 42698 -209
Branches 7945 7928 -17
==========================================
- Hits 22887 22641 -246
- Misses 19214 19255 +41
+ Partials 806 802 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
db96314 to
16d4fb6
Compare
pieqq
left a comment
There was a problem hiding this comment.
Good stuff! I left a few inline suggestions, mainly on naming stuff.
This PR includes plz-run for the snap version, but not for the Debian packages. Do we need to package plz-run in our PPA and pull it when installing Checkbox?
Also document what feature flags are
|
Even though I have ideas as to the future of this and how it may become the default executor for checkbox eventually, currently it is experimental. The only purpose of it right now is to escape snapd/aa sandbox, so we don't need it on debian at all. I have added an error message if one tries to enable it without having the required plz-run binary though |
pieqq
left a comment
There was a problem hiding this comment.
Thanks for the fix and the additional logging information!
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an experimental systemd-based job runner for Checkbox that addresses permission issues with rfkill and provides a foundation for escaping the snap/AppArmor sandbox. The new runner executes jobs as systemd units rather than child processes, which can help bypass certain permission restrictions in strict snap environments.
- Adds a new systemd-based job execution mechanism using plz-run utility
- Implements feature flag system to enable/disable the experimental runner
- Adds comprehensive test coverage for both execution methods
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| checkbox-ng/plainbox/impl/execution.py | Core implementation of systemd unit execution logic and refactored job execution methods |
| checkbox-ng/plainbox/impl/session/state.py | Added feature flag for systemd-based runner with dependency validation |
| checkbox-ng/plainbox/impl/session/assistant.py | Integration of systemd runner flag into job execution flow |
| checkbox-ng/plainbox/impl/test_execution.py | Comprehensive unit tests for both execution methods |
| checkbox-ng/plainbox/impl/config.py | Configuration option for systemd-based runner feature |
| checkbox-ng/checkbox_ng/launcher/subcommands.py | Minor refactoring to use normal user utility function |
| checkbox-core-snap/*/snap/snapcraft.yaml | Added plz-run dependency to all snap configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This introduces a new experimental systemd-based runner. This new runner adresses in the short term the rfkill permission denied situation but the objective is to make it, in the long run, the default runner for checkbox strict frontends.
For now this new runner is disabled by default and clearly marked as experimental when enabled.
Resolved issues
Fixes: #1211
Fixes: https://warthogs.atlassian.net/browse/CHECKBOX-2003
Documentation
New runner clearly documented in the docstring
Tests
New function is unit tested.
Tested on RPI4 on core24, This can be tested in any environment that has plz-run installed. Currently, only snap packaging is supported.
To test this use the following launcher:
The result must be unchanged, the runner should be completely transparent to the user.
Snap builds: