[apparmor] [patch] Add severity() to BaseRule class

[Christian Boltz]

Hello,

Am Freitag, 5. Juni 2015 schrieb Steve Beattie:
> On Wed, Jun 03, 2015 at 11:57:00PM +0200, Christian Boltz wrote:

> > [ 19-baserule-add-severity.diff ]
> 
> Acked-by: Steve Beattie <steve at nxnw.org> as is. Some comments:
> > === modified file utils/apparmor/rule/__init__.py

> > +    def severity(self, sev_db):
> > +        '''return severity of this rule (a number between 0 and 10,
> > where 0 means harmless and 10 means critical), +           or
> > sev_db.NOT_IMPLEMENTED if no severity check is implemented for this
> > rule type.
> Should the comment here say anything about possibly returning
> sev_db.unknown?

Should I add another line to the comment mentioning "unknown"?
(to make things more interesting, the exact value of "unknown" is 
specified when creating a sev_db instance, the default for unknown is 
"10", and everywhere we use sev_db we have _("unknown") as default)

> > === modified file utils/test/test-baserule.py

> > +    def test_default_severity(self):
> > +        sev_db = severity.Severity('severity.db', 'unknown')
> > +        obj = BaseRule()
> > +        rank = obj.severity(sev_db)
> 
> From an object-oriented perspective, it feels a little odd to me to
> ask a rule what the severity of that rule is given a severity db. 

I see the severity as a property of the rule, which is one of the 
reasons why I placed it in the rule. (sev_db is just the tool helping to 
find out the severity, and the main reason why I pass in the sev_db is 
that I don't want to have it as global variable somewhere.)

Another reason was that the check needs to access a class-internal 
variable, and it felt more natural to have access to that variables 
inside the class (and then ask sev_db about a string).

Summed up "Dear rule, how severe are you?"

> It
> seems like the more natural conceptual structure would be to ask the
> severity db what it thinks the severity of a given rule object is. Not
> an objection to this patch, but just one of those things that feels
> quirky about the division of responsibilities.

Summed up: "Dear sev_db, how severe is this rule?"

OK, that sounds like a reason to move the code to severity.py.
(_One_ reason. I have two *eg*)

Can we agree to disagree, or do we need to vote in the next meeting? ;-)


Regards,

Christian Boltz
-- 
> My personal opinion is that bugzilla should be sent in a rocket
> to the oblivion and pick up something sane and usable, [...]
Do you really mean the core Bugzilla software or perhaps some
infrastructure software like this "Internet in CHAINs" thingy or
whatever its actual name is?
[> Cristian Rodríguez and Johannes Meixner in opensuse-factory]