Add UI for managing Qubes update repositories #172
Conversation
|
TODO is removed since I fixed the underlying bug in the other PR. This should be ready to merge. |
|
Happy to squash the remaining PyLint warnings... but it seems unnecessary? The way it's currently written seems pretty clear and it's more concise. |
|
While too long function name indeed is pylint being picky (you can add |
strugee
force-pushed
the
repo-gui
branch
2 times, most recently
from
ce3a552
to
a443e33
Compare
|
@marmarek done! I squashed the changes into the existing PyLint commit. FWIW since I forgot to mention this last night, I have successfully tested these changes (including those I just pushed) decently thoroughly in dom0. |
|
@marmarek CI is passing. |
|
@marmarek ping for review, if you get a chance. No rush. |
|
I haven't forgot, but first I want to get R4.1 in testable state. It's almost there: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.1&build=4.1-20190428&groupid=1 (see that one green dot!) |
First of all, the method that was being called has asserts of its own, so it's guaranteed to either succeed or crash the program with AssertionError. Second, asserts are optimized out by the interpreter when -O is passed. Therefore, this code is buggy because it sometimes wouldn't be run, but we need the side effects.
|
@marmarek thanks very much for the helpful review, and sorry it took me a while to get around to making the changes - everything should be addressed. I also rebased while I was at it. The new changes have been manually tested so this should be ready to go in as soon as CI passes 👍 |
Apparently Python exceptions don't take **kwargs, so we just pass a dictionary as the second (regular) argument. While we're at it, we pretty-print said dictionary when displaying error messages.
Before this change, "foobar\n" on stderr would be rendered (in the warning dialog) as "b'foobar\n'", which is ugly. Now it'll be rendered just as "foobar", followed by an actual newline character.
|
OK, hopefully Travis is fixed (PyLint locally is complaining about short variable names, but it's in code I didn't touch recently... so we'll have to see what Travis says). I also found and fixed some bugs with the error handling, which is now fully tested. Here's what it looks like:
Also, before I forget, I want to point out that the program now unconditionally sets repository settings, even if they haven't changed. It was easier to write that way although if it's a big problem I can make it check beforehand. |
No, that will be problematic, may unintentionally change setting that wasn't touched. And also will fail in case of strict qrexec policy (for example no repository change allowed, user wanted to change just default netvm). |
This definitely worked before, and I haven't touched this code recently so I have no idea why these errors are just now popping up... but whatever.
This helps in situations where the qrexec calls are forbidden; it also prevents settings from being unintentionally changed and gives a noticeable performance boost when the "OK" button is clicked.
|
@marmarek fixed! |
|
Side note: PyQt really messes up debugging under Pdb; I used the method mentioned in this Stack Overflow answer to fix it. According to the mailing list post linked in that answer it's always safe to call that method, so I wonder if we should just call it at application startup to improve developer experience. (I haven't really looked closely into what this method does, yet.) Also, as a note to myself, I don't know if the functions that are now inside a class should be using Edit: here are docs on the method. I'm not really sure how this would impact us but since we don't have a REPL normally (except when debugging) it seems like a good idea? |
|
Thanks @marmarek!! Super pumped to see this in a release :-) |
Screenshot:
Menu options provided:
I'll fix the TODO in the PR with the qrexec services and then delete the hack in this code. But since the rest of this is complete I figured I'd open a PR and get the ball rolling on code review.
Depends on QubesOS/qubes-core-admin-linux#48
Fixes QubesOS/qubes-issues#4550