usbguard: simpler implementation #1707

maximbaz · 2019-02-09T22:29:48Z

Addressing usbguard module as per discussions in #1566 and #1656, this reduces the functionality to what @cyrinux and I actually use in this module. Let's extend this module on specific user requests.

This also fixes #1656, module is finally refreshing instantly and yet we are able to return to "cached_until": self.py3.CACHE_FOREVER.

The diff is a bit simpler if you disable whitespace changes (?w=1).

lasers

Untested. Faithfully approved. 🙏 Recalling / commenting.

lasers · 2019-02-09T23:08:35Z

py3status/modules/usbguard.py

-```
-
-@author Cyril Levis (@cyrinux)
+@author @cyrinux, @lasers, @maximbaz


Over 200 lines deleted. Plz take me off the list. Thx. Take the credit for simple usbguard. :-)

A lot is still based and inspired by you, especially Gio stuff was completely new for me. And now your are helping too. You certainly deserve the credit, are you sure you want to be removed?

Yes, please. I don't want to try and put my name everywhere. If it wasn't for the initial author, we wouldn't have this module. I just helped with getting things moving for initial author. I'm removing myself from your xrandr_rotate module too in cleanup 2 PR.

Your wish is my command

lasers · 2019-02-09T23:09:59Z

py3status/modules/usbguard.py

-    {'full_text': 'Block', 'color': '#ffff00'},
+    {'full_text': '[Reject] ', 'index': '22/reject', 'separator': False, 'separator_block_width': 0, 'urgent': True, 'cached_until': -1},
+    {'full_text': 'USB Flash Drive ', 'separator': False, 'separator_block_width': 0, 'urgent': True},
+    {'full_text': '[Allow]', 'index': '22/allow', 'urgent': True}


Remove unused keys. index, separator, separator_block_width.

Yeah I was not sure how you generate this comment, I ran python usbguard.py and this was printed with default config. Done.

Have you noticed, by the way, that cached_until was assigned only to the first object, not to all of them? Is that by design?

Yeah I was not sure how you generate this comment, I ran python usbguard.py and this was printed with default config.

Same way. cached_until is not needed too. I missed this.

Have you noticed, by the way, that cached_until was assigned only to the first object, not to all of them? Is that by design?

Maybe. I assume this.

py3status/py3status/module.py

Lines 960 to 970 in 624c4d6

# update method object cache

if "cached_until" in result:

cached_until = result["cached_until"]

# remove this so we can check later for output changes

del result["cached_until"]

else:

# get module default cached_until

cached_until = self.module_class.py3.time_in()

my_method["cached_until"] = cached_until

if not cache_time or cached_until < cache_time:

cache_time = cached_until

Removed, but my question was more about actual behavior of running python usbguard.py, is this normal that cached_until attribute is added only to the first object instead of all 3?

UPDATE: I see 🙂

lasers · 2019-02-09T23:17:39Z

py3status/modules/usbguard.py

+        for device_id, string in raw_devices:
+            device = {"id": device_id}
+            for name, regex in self.keys:
+                value = regex.findall(string) or None


I wanted to try re.compile(a single long string) instead of a list of keys to re.compile and maybe encode/decode the string first too.

I'd like to avoid a single huge regex, because it will be difficult to understand it. Encode/decode string beforehand is nice idea, done.

Long regex string should look like this.. and is similar to list, just faster.

regex_string = ( 'first_part (123312)' 'second_part (123312)' 'third_part (123312)' 'fourth_part (123312)' 'fifth_part (123312)' )

But downsides are that this begins to depend on the order of keys in the string we search, and also we need to carefully treat the separators between the strings (e.g. .*, but maybe something more special for multiline strings)... Not sure the speed difference is worth it tbh.

Well, we don't plug/unplug usb devices often so we can live with this as-is. 👍

lasers · 2019-02-09T23:22:28Z

py3status/modules/usbguard.py

-        self.py3.notify_user(
-            msg=format_notification_message,
-            title=format_notification_title,
-            icon="/usr/share/icons/hicolor/scalable/apps/usbguard-icon.svg",


RIP awesome usbguard notification system on dunst. I liked this feature. 🍺

Yeah, but both me and cyrinux disabled it, so postponing until someone actually requests it. But we know where to copy the code, if it is needed 😉

lasers · 2019-02-09T23:24:22Z

py3status/modules/usbguard.py

-            "format_button_device": format_button_device,
-            "format_device_list": format_device_list,
+            "device": len(self.devices),
+            "format_device": self._format_device(self.devices),


Maybe you don't need self. part. Is contained here?

You are right, updated 👍

lasers · 2019-02-09T23:26:34Z

py3status/modules/usbguard.py

-        index = event["index"]
-        button = event["button"]
-        if isinstance(index, int):
+        if not isinstance(event["index"], str):


IIRC str not supported on Python2.

I was about to loudly "booooo", but this time we are in luck:

❯ python2 Python 2.7.15 (default, Jan 10 2019, 23:20:52) [GCC 8.2.1 20181127] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> s = 'a string' >>> if isinstance(s, str): ... print('supported!!!') ... supported!!! >>>

diff --git a/py3status/modules/static_string.py b/py3status/modules/static_string.py index 1cfbfed..6f06381 100644 --- a/py3status/modules/static_string.py +++ b/py3status/modules/static_string.py @@ -17,9 +17,10 @@ class Py3status: """ # available configuration parameters - format = "Hello, world!" + format = u"_▁▂▃▄▅▆▇█" def static_string(self): + self.py3.log(isinstance(self.format, str)) return { "cached_until": self.py3.CACHE_FOREVER, "full_text": self.py3.safe_format(self.format),

$ python3 static_string.py Module `test_module`: True {'full_text': '_▁▂▃▄▅▆▇█', 'cached_until': -1} ^C% $ python2 static_string.py Module `test_module`: False {'full_text': u'_\u2581\u2582\u2583\u2584\u2585\u2586\u2587\u2588', 'cached_until': -1} ^C%

OMG, boooooooo python2! I hate it, kills so many cool ideas :(

Will revert to a less error-prone int comparison in a second.

lasers · 2019-02-09T23:28:17Z

py3status/modules/usbguard.py

+        device_id, policy_name = event["index"].split("/")
+        device_id = int(device_id)
+        policy = self.init["target"][policy_name]
+        self.proxy.applyDevicePolicy("(uub)", device_id, policy, False)


Cause we remove notification, I guess you can int(device_id) to kill a line. Just saying.

True, done :)

lasers · 2019-02-09T23:31:25Z

py3status/modules/usbguard.py

+                    value = value.encode("latin-1").decode("utf-8")
+                device[name] = value
+            if device["policy"] == "block":
+                devices.append(device)


Can you not use this instead? raw_devices = self.proxy.listDevices("(s)", "block").

What happen if users change default policy in usbguard config?

Wow, I didn't know this - awesome! Done.

We will parameterize if there is at least one person who wants to use a different policy. It's my understanding however that among usbguard users "block" generally means "temporarily block until decision is taken" and "reject" means "I've decided, don't want this device", so if someone complains, before rushing to implement a parameter I would like to find out why they changed the default policy.

IIRC there is a permanent flag that you also can enable in Qt or via config to default block either temporary or permanently, but if it's rejected permanently, it never can be allowed again until it is removed from the filters (which is the long route)... meaning if users accidentally rejected or on purpose rejected something, then they might wonder why some device isn't working again weeks or months later if they plugged in again.

Edit: Could be abrasive to default format and no block to replace reject with.

That's right, "permanent" is another beast, it remembers your choice and stores it in a file. We chatted with cyrinux when he was testing usbguard from this branch, it was a conscious decision to remove "permanent" flag and let users handle "permanent" actions via CLI or other tools, because the cost of mistake (pressing wrong button accidentally) is high, and you better be sure what you are doing. We can reconsider in the future if there will be demand for this option of course, but I don't think it's a good idea to add it now.

lasers · 2019-02-09T23:41:11Z

py3status/modules/usbguard.py

+        device_info = []
+        for device in devices:
+            for btn in self.init["format_button"]:
+                composite = self.py3.safe_format(getattr(self, btn), device)


I think you're killing off format_button_block. Is it not a useful button for users? You could do (getattr(self, btn, '{{{}}}'.format{btn), device) because I think users using {format_button_block} would be broken at this part. The code changes it to printing {button_button_block}.

Is it better? I dunno. Users would have to change their config either way. Module breakage vs ugly. Just saying.

It is intentional decision to keep only two buttons, "approve" and "reject". As I explained above, "blocked" devices are assumed to be our "TODO items" that need an approval, so a "block" button would literally be a noop.

lasers · 2019-02-09T23:43:04Z

py3status/modules/usbguard.py

-        '|\?color=darkgray Permanently]')*
-    format_button_reject: display format for reject button
-        (default '\?color=bad Reject')
+        (default '[{format_device}]')


Do you need [] around here? With block, I assume nothing get displayed now.

Works equally well, updated 👍 Nothing gets displayed if there are no blocked devices, that's right.

maximbaz added 2 commits February 9, 2019 23:23

usbguard: simpler implementation

9630beb

Make Travis happy

90729a5

maximbaz force-pushed the simple-usbguard branch from 2da611c to 90729a5 Compare February 9, 2019 22:37

allow_urgent is supported by core

48e0c2d

lasers approved these changes Feb 9, 2019

View reviewed changes

maximbaz added 3 commits February 10, 2019 01:15

Review feedback

6c6b663

Removing lasers, updating sample output

6051e3b

python2 :(

6aa3363

ultrabug approved these changes Feb 10, 2019

View reviewed changes

ultrabug merged commit 69688ff into master Feb 10, 2019

lasers deleted the simple-usbguard branch February 10, 2019 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

usbguard: simpler implementation #1707

usbguard: simpler implementation #1707

maximbaz commented Feb 9, 2019

lasers left a comment

lasers Feb 9, 2019

maximbaz Feb 9, 2019

lasers Feb 10, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 10, 2019 •

edited

Loading

maximbaz Feb 10, 2019 •

edited

Loading

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 10, 2019 •

edited

Loading

maximbaz Feb 10, 2019

lasers Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 10, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 10, 2019 •

edited

Loading

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 10, 2019

lasers Feb 9, 2019

maximbaz Feb 9, 2019

	# update method object cache
	if "cached_until" in result:
	cached_until = result["cached_until"]
	# remove this so we can check later for output changes
	del result["cached_until"]
	else:
	# get module default cached_until
	cached_until = self.module_class.py3.time_in()
	my_method["cached_until"] = cached_until
	if not cache_time or cached_until < cache_time:
	cache_time = cached_until

usbguard: simpler implementation #1707

usbguard: simpler implementation #1707

Conversation

maximbaz commented Feb 9, 2019

lasers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasers Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

maximbaz Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasers Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasers Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasers Feb 10, 2019 •

edited

Loading

maximbaz Feb 10, 2019 •

edited

Loading

lasers Feb 10, 2019 •

edited

Loading

lasers Feb 10, 2019 •

edited

Loading