[pkg-apparmor] UDD notifications script

intrigeri intrigeri at debian.org
Sun Mar 1 16:29:28 UTC 2015


Hi,

u wrote (23 Feb 2015 11:29:13 GMT) :
> thank you both Christian and intrigeri for the valuable input.
> I'm having a lot of fun implementing the corrections :)

:)

> intrigeri:
>> * This looks suboptimal (but of course, as long as we haven't
>>   a massive amount of usertagged bugs, it should be just fine):
>> 
>>  	for bug in cursor.fetchall():
>> 		buglist.append(bug)
>> 
>>   ... so I'm curious: can't we do that in one batch operation instead?

> Done, using Christian's suggestion.

Great!

I believe that using a list comprehension like (untested):

    return [ {'id': item[0], 'tag': item[1], 'title': item[2]}
             for item in items ]

(cute, isn't it?)

... would be more Pythonic than:

    buglist = []
    for item in items:
        bug = {'id': item[0], 'tag': item[1], 'title': item[2]}
        buglist.append(bug)

    return buglist

I think I would even get rid of the items temporary variable, and
instead write:

    return [ {'id': item[0], 'tag': item[1], 'title': item[2]}
             for item in cursor.fetchall() ]

Similarly:

    for item in new_state_data:
        if item not in old_state_data:
            added_usertags.append(item)

... could be made more declarative, if you fancy declarative code (as
opposed to procedural one) as much as I do:

    added_usertags = [ item for item in new_state_data
                       if item not in old_state_data ]

(same for deleted_usertags)

> I finally did that by joining the two tables and it the query still
> seems to be done quite quickly.

Yay!

>> * pylint says that "def get_bug_title(bugid) :" has one too much
>>   space. It's also quite unhappy about a lot of other small things.
>>   Possibly it knows Python best practices better than you and I.
>>   I've no idea (really -- it might produce obsolete recommendations,
>>   and I've not checked). Similarly, pyflakes is a bit mean and detects
>>   two unused variables.

> I will run this on the new script too.

Cool. It's now unhappy with using strings as comments outside of
a function's docstring. And indeed, I kinda emphatize with it.

>> * 'print "No new data."' might produce useless noise when run from
>>   cron, no?

> Deleted.

I since realized (too late) that we could simply use chronic (from the
moreutils package), and then output whatever you feel is useful when
working on the script.


And congrats for the numerous improvements I'm snipping from my
reply! :)

Cheers,
-- 
intrigeri



More information about the pkg-apparmor-team mailing list