-
-
Notifications
You must be signed in to change notification settings - Fork 115
Introduce LocalVM to extract BaseVM being smaller class #637
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
|
@marmarek can you run openQA on it please? |
|
Sure, I can, but I think you may want to first take care of unit tests, this one looks like a real issue: |
0e2e8d9 to
9b8407f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #637 +/- ##
==========================================
+ Coverage 70.14% 70.24% +0.09%
==========================================
Files 59 59
Lines 12726 12739 +13
==========================================
+ Hits 8927 8948 +21
+ Misses 3799 3791 -8
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:
|
Fixed. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025032603-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025031804-4.3&flavor=update
Failed tests22 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/132953#dependencies 14 fixed
Unstable testsPerformance TestsPerformance degradation:13 performance degradations
Remaining performance tests:59 tests
|
It prepares the work for RemoteVM.
qubes/vm/__init__.py
Outdated
| `domain-init` and `domain-load` events. | ||
| """ | ||
|
|
||
| # pylint: disable=no-member |
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.
Why? it is defined in __init__
qubes/vm/__init__.py
Outdated
| import qubesdb # pylint: disable=import-error | ||
|
|
||
| try: | ||
| # pylint: disable=no-member |
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.
same here
qubes/vm/__init__.py
Outdated
| class. | ||
| """ | ||
| # cleanup old watch connection first, if any | ||
| # pylint: disable=no-member |
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.
and here
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.
I don't remember, I double checked, now it's good locally. I removed the three disable.
qubes/vm/__init__.py
Outdated
| ) | ||
| ) | ||
| if not issubclass(vmclass, BaseVM): | ||
| if not issubclass(vmclass, LocalVM): |
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.
While default to LocalVM makes sense, should it really forbid using RemoteVM as some property value?
qubes/vm/adminvm.py
Outdated
|
|
||
| def __lt__(self, other: object): | ||
| if not isinstance(other, BaseVM): | ||
| if not isinstance(other, LocalVM): |
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.
This should remain BaseVM, otherwise sorting VM objects will not work with RemoteVM
|
PipelineRetry |
It prepares the work for RemoteVM.