-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Migration to View/Model design #195
Conversation
Heads up: I've just merged #178 which most likely conflicts with this one. Also, As for the actual change, generally sounds good (I haven't looked into details, as some of it isn't finished yet). Please keep precise data updates working - I mean if some property is changed (and you get an event about it), refresh only that property, not the whole VM. |
No problems with that @marmarek . I am working on it but unfortunately I am having problems with 'State' column and his multiple icons/tooltips. I asked for some help at pyqt mail list, I hope to solve it soon. |
It affects resizeColumnsToContents()
Used for paint different icons on same cell with custom tooltips.
It reacts to selection changes but it is missing real functionally yet.
Still working on this, unfortunately I had pretty difficult days and I could not work on this. I merged with PyQt5 version, solved my problem with multiple icons in a same cell (thanks to pyqt guys), and started the multi-row support. |
indexes need proxy.mapToSource(index) model.layoutChanged.emit(), replaced by proxy.invalidate()
After reading official pyqt doc this seems the standard way
After reading official pyqt doc this seems the standard way (Continue previous commit)
qubesmanager/qube_manager.py
Outdated
ret = self.data(index, Qt.DisplayRole) | ||
|
||
if col_name != "Name": | ||
ret = ret + vm.name |
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.
The tests checks if dom0 is sorted first, but this doesn't guarantee it. In fact, many of those tests fails for me. For example when sorting by "Template", it gets dom0
vs debian-10-xfcesys-gui
and the latter is sorted earlier.
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.
Oh, it was ' return "" ' for dom0 and worked fine but I changed it fixing a pylint warning 🤣
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 concatenation has other unintended effects:
======================================================================
FAIL: test_100_sorting (qubesmanager.tests.test_qube_manager.QubeManagerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python3.8/site-packages/qubesmanager/tests/test_qube_manager.py", line 251, in test_100_sorting
self.__check_sorting("Template")
File "/usr/lib/python3.8/site-packages/qubesmanager/tests/test_qube_manager.py", line 1266, in __check_sorting
self.assertGreater(
AssertionError: 'fedora-32' not greater than 'fedora-32-xfce' : Incorrect sorting for Template
----------------------------------------------------------------------
That's because it isn't fedora-32
sorted against fedora-32-xfce
- it is fedora-32vmname
sorted against fedora-32-xfcevmname
and indeed this reverses the order here. Adding some separator that cannot appear in the column content (!
? a space?) should help, but still feels hacky. Perhaps use tuples (primary value, vmname) to make sure the vm name is used for sorting really only when the first value matches?
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 tried with a better way, or a least not so hacky.
That's because you've changed how QMessageBox is imported. When you do |
I think that the problem is that there is no row selected so 'action' is not enabled and trigger() does nothing. I would try to select the 'test-vm' row but I don't understand this lines:
I manually created both 'test-vm' (providing network) and 'test_prop' with test-vm as net-vm. Should be enough to select test-vm before calling trigger()? |
Yes, that's indeed the other thing there. Just select some not running VM.
No need - it is mock return value what other VMs depends on the one you're going to remove. Since it's mockup, it doesn't actually need to exist at all. |
This applies to other tests too, specifically VMShutdownMonitorTest class. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/10869#dependencies Failed tests
New failuresCompared to: https://openqa.qubes-os.org/tests/10831#dependencies
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/10831#dependencies
|
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.
Since the change is quite simple and otherwise it is all good, I'm going merge this and fix it myself.
Squashed commit of the following: commit 7929b8f Author: donoban <[email protected]> Date: Tue Jul 28 17:21:11 2020 +0200 Try to fix sort commit 5e4598e Author: donoban <[email protected]> Date: Mon Jul 27 04:06:37 2020 +0200 Fix import commit 60f53e7 Author: donoban <[email protected]> Date: Mon Jul 27 04:03:17 2020 +0200 Fix 218 test commit e430e39 Author: donoban <[email protected]> Date: Mon Jul 27 04:01:56 2020 +0200 Avoid error if dvm is None commit 679880f Author: donoban <[email protected]> Date: Mon Jul 27 03:58:59 2020 +0200 Fix sorting again commit f84edcd Author: donoban <[email protected]> Date: Sun Jul 26 03:30:14 2020 +0200 Yes, it's needed commit 5d00c91 Author: donoban <[email protected]> Date: Sat Jul 25 23:08:38 2020 +0200 Fix pylint error commit 88a54dc Author: donoban <[email protected]> Date: Sat Jul 25 18:56:40 2020 +0200 Style change commit 42ae96c Author: donoban <[email protected]> Date: Sat Jul 25 18:56:00 2020 +0200 Fix sorting errores commit daa8722 Author: donoban <[email protected]> Date: Sat Jul 25 18:54:21 2020 +0200 Fixed sort test errors commit 73ad25e Author: donoban <[email protected]> Date: Sat Jul 25 00:25:37 2020 +0200 Var rename commit 825d8ad Author: donoban <[email protected]> Date: Fri Jul 24 23:37:04 2020 +0200 Restored Cleanup commit 09f1839 Author: donoban <[email protected]> Date: Fri Jul 24 23:35:20 2020 +0200 Removed workaround, now works properly without clear reason commit 2f5bde0 Author: donoban <[email protected]> Date: Fri Jul 24 23:29:20 2020 +0200 Multiple tests fixes commit e21f9ab Author: donoban <[email protected]> Date: Fri Jul 24 23:28:32 2020 +0200 Save dvm name instead VM object commit 46e2fe1 Author: donoban <[email protected]> Date: Fri Jul 24 01:11:15 2020 +0200 Deleted wrong mapToSource() commit b155e05 Author: donoban <[email protected]> Date: Fri Jul 24 01:10:43 2020 +0200 Fix get 'Is DVM Template' widget commit 61d7a6d Author: donoban <[email protected]> Date: Wed Jul 22 12:17:10 2020 +0200 fix set_keyboar_layout test fail commit 1dba52e Author: donoban <[email protected]> Date: Sun Jul 19 00:05:53 2020 +0200 More test fixes commit 665a145 Author: donoban <[email protected]> Date: Sun Jul 12 23:39:07 2020 +0200 Fixed power state checking commit 6733fb1 Author: donoban <[email protected]> Date: Sun Jul 12 17:44:01 2020 +0200 Return vm object instead name on select_vm functions commit 80f3b3f Author: donoban <[email protected]> Date: Sun Jul 12 17:43:27 2020 +0200 Removed wrong calls to text() commit 32bbb86 Author: donoban <[email protected]> Date: Sun Jul 12 17:42:45 2020 +0200 Removed implicity calls to sortItems() commit bc288b6 Author: donoban <[email protected]> Date: Sun Jul 12 17:03:22 2020 +0200 setCurrentItem() -> setCurrentIndex() commit 10bac8d Author: donoban <[email protected]> Date: Sun Jul 12 16:43:41 2020 +0200 get_table_vminfo renamed to get_table_vm commit cee7b0a Author: donoban <[email protected]> Date: Sat Jul 11 23:46:41 2020 +0200 First version fixing tests commit 42d566f Author: donoban <[email protected]> Date: Sat Jul 11 23:38:33 2020 +0200 Fixing tests commit ccd7c16 Merge: 24e5d58 8a74e43 Author: donoban <[email protected]> Date: Mon Jun 8 22:16:34 2020 +0200 Merge branch 'master' of https://github.com/QubesOS/qubes-manager # Conflicts: # qubesmanager/qube_manager.py commit 24e5d58 Author: donoban <[email protected]> Date: Sun Jun 7 19:03:09 2020 +0200 Added workaround for dom0 sorting commit db2781a Author: donoban <[email protected]> Date: Sun Jun 7 18:57:28 2020 +0200 Fixed Sorting Case Insensivity commit 93330ea Author: donoban <[email protected]> Date: Sun Jun 7 18:51:39 2020 +0200 Added "default" to netvm and default dispvm commit a40156c Author: donoban <[email protected]> Date: Sun Jun 7 18:18:03 2020 +0200 Fixed QSettings saving commit a1d96e7 Author: donoban <[email protected]> Date: Wed Jun 3 00:23:50 2020 +0200 Added 'defaultValue' on settings load commit a0a7ee8 Author: donoban <[email protected]> Date: Wed Jun 3 00:10:31 2020 +0200 Init view menu out of load_manager_settings commit 6f9a600 Author: donoban <[email protected]> Date: Tue Jun 2 23:19:09 2020 +0200 "Size" renamed to "Disk Usage" commit 5fbda06 Author: donoban <[email protected]> Date: Tue Jun 2 01:34:56 2020 +0200 Replaced unneded elif's with if's commit 5516bca Author: donoban <[email protected]> Date: Tue Jun 2 01:30:46 2020 +0200 Use "Yes"/"" for bool properties commit 1e5429e Author: donoban <[email protected]> Date: Tue Jun 2 01:12:46 2020 +0200 Restored exactly old icon size commit 270c825 Author: donoban <[email protected]> Date: Sun May 31 12:52:03 2020 +0200 AdminVM and DispVM icon workaround commit cfb8a87 Author: donoban <[email protected]> Date: Sun May 31 12:51:44 2020 +0200 Icon size adjusted to 128/4 commit 173dc94 Author: donoban <[email protected]> Date: Sat May 30 00:56:40 2020 +0200 Add italic and gray color for differentiate templates and standalone/dom0 commit 2062f93 Author: donoban <[email protected]> Date: Thu May 28 00:21:58 2020 +0200 Fixig Marek comments commit 348485e Author: donoban <[email protected]> Date: Thu May 28 00:03:44 2020 +0200 More readable commit dc823a3 Author: donoban <[email protected]> Date: Thu May 28 00:01:02 2020 +0200 Needed for pylint proplerly import PyQt5 modules on fedora 32 commit 4478b28 Author: donoban <[email protected]> Date: Tue May 19 01:11:05 2020 +0200 Removed unused unued vars commit 450f0e3 Author: donoban <[email protected]> Date: Fri May 8 00:26:59 2020 +0200 Fix wrong var names commit c1bd957 Author: donoban <[email protected]> Date: Fri May 8 00:24:31 2020 +0200 Fixed params order to VmSettingsWindow() commit 6d50d03 Author: donoban <[email protected]> Date: Fri May 8 00:20:06 2020 +0200 Modeless settings windows commit ef3ac6a Author: donoban <[email protected]> Date: Thu May 7 23:51:30 2020 +0200 Fix some vm/vm_info confusion commit 09392f9 Author: donoban <[email protected]> Date: Wed Apr 29 10:26:58 2020 +0200 removed trailing whitespace commit 9e35ddf Author: donoban <[email protected]> Date: Wed Apr 29 00:50:27 2020 +0200 columns_indices redudancy fixed and menu_view auto generation commit 8d96ef4 Author: donoban <[email protected]> Date: Sat Apr 25 00:29:53 2020 +0200 Use col_name instead col number, improves readiblity commit 1cae3ca Author: donoban <[email protected]> Date: Fri Apr 24 00:52:12 2020 +0200 Add QubesNoSuchProperyError commit aed771d Author: donoban <[email protected]> Date: Fri Apr 24 00:45:59 2020 +0200 Added missing virt_mode commit 580749b Merge: 70878dc b058db4 Author: donoban <[email protected]> Date: Fri Apr 24 00:16:48 2020 +0200 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 70878dc Author: donoban <[email protected]> Date: Fri Apr 24 00:16:31 2020 +0200 Let's try travis commit 5f65477 Author: donoban <[email protected]> Date: Fri Apr 24 00:11:37 2020 +0200 Fix ProgressDialog not being properly drawn commit b577cb9 Author: donoban <[email protected]> Date: Sun Apr 12 23:44:27 2020 +0200 pylint fixes and wrong 'outdated' commit 2a55c5d Author: donoban <[email protected]> Date: Sun Apr 12 23:35:47 2020 +0200 Restored menubar and toolbar context menu commit ac70860 Author: donoban <[email protected]> Date: Sun Apr 12 23:28:02 2020 +0200 restored logs commit a0b2b7b Author: donoban <[email protected]> Date: Sun Apr 12 23:16:03 2020 +0200 Removed unused attributes commit cb51494 Author: donoban <[email protected]> Date: Sun Apr 12 23:08:00 2020 +0200 Part of last commit... commit 7f0c42f Author: donoban <[email protected]> Date: Sun Apr 12 23:07:20 2020 +0200 Save sort settings on closeEvent commit 8dcfc3c Author: donoban <[email protected]> Date: Sun Apr 12 13:02:37 2020 +0200 Pylint fixes commit 8e5f9ff Author: donoban <[email protected]> Date: Mon Apr 6 23:35:15 2020 +0200 State converted to dict making pylint happier commit 233ec12 Author: donoban <[email protected]> Date: Mon Apr 6 00:25:34 2020 +0200 Pylint fixes commit 37790f0 Author: donoban <[email protected]> Date: Sun Apr 5 23:47:17 2020 +0200 pylint commit 7dbe393 Author: donoban <[email protected]> Date: Sun Apr 5 23:41:12 2020 +0200 pylint fixes commit f79f096 Author: donoban <[email protected]> Date: Sun Apr 5 23:37:03 2020 +0200 fixed wrong info_by_id refrences commit dbf17bd Author: donoban <[email protected]> Date: Sun Apr 5 17:46:31 2020 +0200 Added QubesCache QubesTableModel and main app should operate directly to the cache commit 42d1245 Author: donoban <[email protected]> Date: Sun Apr 5 13:50:00 2020 +0200 Fixing multiple pylint warnings commit c708b42 Author: donoban <[email protected]> Date: Sun Apr 5 12:59:43 2020 +0200 Added action_open_console setEnabled commit de14994 Author: donoban <[email protected]> Date: Fri Apr 3 00:25:07 2020 +0200 Forgot context_menu.actions() commit d24903b Author: donoban <[email protected]> Date: Fri Apr 3 00:03:16 2020 +0200 Elegant alternative for _enable_all() commit a060387 Author: donoban <[email protected]> Date: Thu Apr 2 00:22:46 2020 +0200 Fixed outdate commit 36e4b31 Author: donoban <[email protected]> Date: Wed Apr 1 11:36:28 2020 +0200 Removed table_widgets.py dependency commit 72e679e Author: donoban <[email protected]> Date: Wed Apr 1 00:55:10 2020 +0200 Fixed pylint warnings commit 8e118be Author: donoban <[email protected]> Date: Wed Apr 1 00:26:14 2020 +0200 Added get_selected_vms() and UserRole + 1 commit fd12a95 Author: donoban <[email protected]> Date: Tue Mar 31 01:10:51 2020 +0200 fix some pylint warnings commit 09dfe83 Author: donoban <[email protected]> Date: Tue Mar 31 00:34:51 2020 +0200 Removed unneded margins commit f0c81bf Merge: 00876bc f1ad829 Author: donoban <[email protected]> Date: Tue Mar 31 00:33:53 2020 +0200 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 00876bc Author: donoban <[email protected]> Date: Mon Mar 30 23:31:18 2020 +0200 Alternative pyqt imports After reading official pyqt doc this seems the standard way (Continue previous commit) commit 6cf09d3 Author: donoban <[email protected]> Date: Mon Mar 30 23:29:21 2020 +0200 Alternative pyqt imports After reading official pyqt doc this seems the standard way commit 410dbae Author: donoban <[email protected]> Date: Mon Mar 30 00:12:09 2020 +0200 Restored sorting and filtering using QSortFilterProxyModel() commit 0b7fd6e Author: donoban <[email protected]> Date: Tue Mar 24 12:46:18 2020 +0100 Added QSortFilterProyModel indexes need proxy.mapToSource(index) model.layoutChanged.emit(), replaced by proxy.invalidate() commit 97440e8 Author: donoban <[email protected]> Date: Tue Mar 24 12:34:35 2020 +0100 Removed unneded calls to setContentsMargins commit 1ad2aaa Author: donoban <[email protected]> Date: Sun Mar 22 22:56:33 2020 +0100 fix removevm with multiselection commit 19be1da Author: donoban <[email protected]> Date: Sun Mar 22 22:34:52 2020 +0100 Restored context menu commit f43394a Author: donoban <[email protected]> Date: Sun Mar 22 00:08:43 2020 +0100 Deleted unedeed updates after change of settings commit c98ba62 Merge: 103c572 cf3f102 Author: donoban <[email protected]> Date: Sat Mar 21 23:45:46 2020 +0100 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 103c572 Merge: 2756864 da2826d Author: donoban <[email protected]> Date: Sat Feb 29 16:40:22 2020 +0100 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 2756864 Merge: 2e2a14b 8902727 Author: donoban <[email protected]> Date: Thu Jan 23 23:43:32 2020 +0100 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 2e2a14b Author: donoban <[email protected]> Date: Wed Jan 8 16:41:30 2020 +0100 Removed fill_table :) commit 9f3f61a Author: donoban <[email protected]> Date: Tue Dec 31 17:29:39 2019 +0100 When Template changes status, all AppVMs should update too commit b970a70 Author: donoban <[email protected]> Date: Fri Dec 27 17:59:05 2019 +0100 Improved multi row system commit 2f3fc98 Merge: 1f21da6 cca5d7d Author: donoban <[email protected]> Date: Fri Dec 27 17:25:15 2019 +0100 Merge remote-tracking branch 'upstream/master' commit 1f21da6 Author: donoban <[email protected]> Date: Mon Sep 23 21:41:39 2019 +0200 Restored 'selection changed' with multiple row support It reacts to selection changes but it is missing real functionally yet. commit bdf1601 Author: donoban <[email protected]> Date: Wed Sep 18 07:27:47 2019 +0200 Restored add/remove/change events handling commit 2f9b21f Author: donoban <[email protected]> Date: Wed Sep 18 07:00:49 2019 +0200 Added StateIconDelegate and StateInfo Used for paint different icons on same cell with custom tooltips. commit ccfa545 Author: donoban <[email protected]> Date: Wed Sep 18 06:35:12 2019 +0200 Removed Default and Minium horizonal header section size It affects resizeColumnsToContents() commit 628073e Author: donoban <[email protected]> Date: Sun Sep 15 10:45:36 2019 +0200 Uncompatible with TableView commit 52ddd56 Merge: 0a87cf9 1ced452 Author: donoban <[email protected]> Date: Sun Sep 15 10:43:13 2019 +0200 Merge branch 'master' of https://github.com/QubesOS/qubes-manager commit 0a87cf9 Author: donoban <[email protected]> Date: Mon Sep 2 21:55:21 2019 +0200 Restored precises updates QubesOS#195 (comment) commit 030bf13 Author: donoban <[email protected]> Date: Sun Aug 25 18:33:11 2019 +0200 New and dirty first Model/View version commit 981ee9c Author: donoban <[email protected]> Date: Sun Aug 25 18:32:28 2019 +0200 QtableWidget > QTableView commit 41beaed Author: donoban <[email protected]> Date: Sun Aug 25 18:31:59 2019 +0200 Removed table_widgets
Merged by squashing all the commits into one to limit noise in the git history. |
Ohh great! |
Ey @marmarek , I would like to fix some sorting behaviour and to add filtering options but my fork repo is 107 commits ahead. I suppose that it's due squashing but I don't know what should I do for fix it. |
If you didn't made any other changes in the repo, you can reset your repo to the upstream state, like this:
For the next time, I strongly recommend doing any changes in a separate branch, not master. |
Ok thanks! |
Hi,
I started to work on a migration to View/Model concept for Qubes Manager. It is in early stage yet but I hope I could finish in few weeks and I want to hear some opinions and comments, also give it some visibility in case another dev starts another pretty big rewrite.
Currently no action is working, it just displays info.
A brief explanation
View/Model tries to maintain separated the data from the code which handles its representation and decoration. In this case, we could just remap the data from a VM object, 'column 2 -> self.vm.name e.g.', but this causes too much overload on qubesd, so I use a class for caching all needed data (VmInfo). The model (QubesTableModel) fills a list of VmInfo objects and it's ready for start drawing the table.
The current version already does some kind of caching, storing needed values on the widgets for speed up sorting. With this version all the cached info is in the same place and there is no duplication.
The other part of the model is on data() method. Here we handle the information displayed and the decoration, also we could handle user input like editing directly a cell of the table.
I am pretty happy with the changes. Although the appearance is not yet identical, it is pretty close and 'table_widgets.py', a place with a lot of complexity, is fully removed. I will restore some code from there, but I don't think a separate file will be needed.
The main objective is to have more readable code and easier to maintain. Also once it's finished amazing features will be pretty simple to add (multiple selection for manage multiple VM's at once, changing options like template/netvm directly on the table with a ComboBox, change table to a tree and have VM's sorted in different groups...). However it also seems to improve performance. Startup process has a pretty big boost, I get from 3.92s to 2s :)