Alon Bar-Lev has posted comments on this change.

Change subject: tools: Support snmp trap as a notification method.
......................................................................


Patch Set 18:

(1 comment)

quick note for configuration.

http://gerrit.ovirt.org/#/c/22909/18/packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.conf.in
File packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.conf.in:

Line 149: #
Line 150: # 3.) add a new snmp subscriber to all events using default 
definitions, overriding only the oid.
Line 151: # SNMP_OID_DAVE=1.2.3.4
Line 152: # FILTER="${FILTER} -(SNMP:DAVE)"
Line 153: FILTER=
this is not what we actually discussed, you tried to optimize the syntax but I 
think it is very complex.

 id,id,id,id(transport)

is very hard to read when there are lots of ids, but if you do that, then I 
prefer something like python tuple:

 (id, id, id, id)(transport), ..

this will group each better.

also if you added the '-' operator, we can remove the DEFAULT_FILTER_MODE.

and... I really prefer include: or exclude: over '-' (if you introduce an 
operator).
Line 154: 
Line 155: #----------------------------------#
Line 156: # Engine Monitoring Configuration: #
Line 157: #----------------------------------#


-- 
To view, visit http://gerrit.ovirt.org/22909
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd22d022ae535f45e046b09a2cbfadd837b465c
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Martin PeÅ™ina <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to