Allow named disposables in OpenInVM config#314
Conversation
cbd2d3c to
f8a4eed
Compare
|
@ben-grande |
| @staticmethod | ||
| def _dvm_template_filter(vm: qubesadmin.vm.QubesVM): | ||
| return getattr(vm, "template_for_dispvms", False) | ||
| def _dispvm_target_filter(vm: qubesadmin.vm.QubesVM): |
There was a problem hiding this comment.
A dvm is not the same as a dispvm. Maybe use _permanent_dispvm_and_dispvm_template_filter?
| if getattr(vm, "template_for_dispvms", False): | ||
| return True | ||
|
|
||
| auto_cleanup = str(getattr(vm, "auto_cleanup", False)).lower() == "true" |
There was a problem hiding this comment.
The value can be compared as bool, no need to convert to str. Why was it compared as str?
| rule = RuleDispVM( | ||
| Rule.from_line( | ||
| None, | ||
| "Service\t*\ttest-blue\t@dispvm\tallow\ttarget=@dispvm", |
There was a problem hiding this comment.
I know that make_rule is using tabs and f-string with vars, but here it is str and the \t makes it hard to read. Use of \t is unnecessary.
|
|
||
| assert not rule_row.target_widget.model.is_vm_available( | ||
| test_qapp.domains["test-disp"] | ||
| ) |
|
There is a new tab called "Disposables", at |
f8a4eed to
cc49496
Compare
|
@ben-grande |
b2d2a1a to
638f161
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #314 +/- ##
==========================================
- Coverage 92.72% 92.70% -0.02%
==========================================
Files 66 66
Lines 13618 13640 +22
==========================================
+ Hits 12627 12645 +18
- Misses 991 995 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Skimmed through. Looks correct. Marta will review as she is responsible for this repo. |
| } | ||
| ), | ||
| is_new_row=new, | ||
| filter_target=self.target_filter, |
There was a problem hiding this comment.
This is minor, but can you use consistent naming? As in, make this "filter_target" everywhere (or rename the parameter to "target_filter", I'm good with either)
| verb_description=verb_description, | ||
| initial_verb=_("will"), | ||
| filter_target=self._dvm_template_filter, | ||
| filter_target=( |
There was a problem hiding this comment.
I don't like this approach. I don't want this function to be hard-coded here, please pass it to the constructor when needed and not hard-code it within this class.
638f161 to
deb1882
Compare
|
I made the naming consistent and moved the custom target filter out of |
Fixes QubesOS/qubes-issues#9709.
What changed
DispVMwithauto_cleanupdisabled) in addition to disposable templates.Validation
python -m py_compile qubes_config\global_config\rule_list_widgets.py qubes_config\tests\test_rule_list_widgets.pygit diff --checkpython -m black ...andpython -m pytest ...could not be run in this Windows checkout because the active Python environment does not haveblackorpytestinstalled.