Alon Bar-Lev has posted comments on this change.

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


Patch Set 22:

(4 comments)

http://gerrit.ovirt.org/#/c/22909/22/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java:

Line 146:                 NotificationProperties.INTERVAL_IN_SECONDS,
Line 147:                 NotificationProperties.IS_HTTPS_PROTOCOL,
Line 148:                 
NotificationProperties.REPEAT_NON_RESPONSIVE_NOTIFICATION);
Line 149:         // validate mandatory and non empty properties
Line 150:         requireOne(NotificationProperties.MAIL_SERVER, 
NotificationProperties.SNMP_MANAGERS);
> Well no. I thought to require the default managers since I don't see use ca
so please support that, or remove constraint.
Line 151: 
Line 152:         // validate non negative args
Line 153:         for (String property : new String[] {
Line 154:                 NotificationProperties.DAYS_TO_KEEP_HISTORY,


Line 198:                             "'%s' must be set when SSL or TLS is 
enabled or when password is set",
Line 199:                             NotificationProperties.MAIL_USER));
Line 200:         }
Line 201: 
Line 202:         if (!isSmtpEncryptionOptionValid()) {
> It does adding snmp I renamed smtp
rename at different patch
Line 203:             throw new IllegalArgumentException(
Line 204:                     String.format(
Line 205:                             GENERIC_MESSAGE + "'%s' value has to be 
one of: '%s', '%s', '%s'.",
Line 206:                             
NotificationProperties.MAIL_SMTP_ENCRYPTION,


Line 241:     }
Line 242: 
Line 243:     private void validateSnmpAvailability() {
Line 244:         for (String snmpManager : 
getProperty(SNMP_MANAGERS).split("\\s+")) {
Line 245:             validateHost(SNMP_MANAGERS, snmpManager.replaceAll(":.*", 
""));
> What I did was to apply the same behavior we had for smtp(and event limit i
it should be gone in my opinion, in both cases.
Line 246:         }
Line 247:     }
Line 248: 
Line 249:     private void validateFilter() {


http://gerrit.ovirt.org/#/c/22909/22/packaging/dbscripts/event_sp.sql
File packaging/dbscripts/event_sp.sql:

Line 19: RETURNS VOID
Line 20:    AS $procedure$
Line 21: BEGIN
Line 22: INSERT INTO event_notification_hist(audit_log_id, event_name, 
method_type, reason, sent_at, status)
Line 23:        VALUES(v_audit_log_id, v_event_name, v_method_type, v_reason, 
v_sent_at, v_status);
> But it does. 
different patch, has nothing to do with snmp
Line 24: END; $procedure$
Line 25: LANGUAGE plpgsql;
Line 26: 
Line 27: 


-- 
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: 22
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