Alon Bar-Lev has posted comments on this change. Change subject: tools: Support snmp trap as a notification method. ......................................................................
Patch Set 27: (3 comments) http://gerrit.ovirt.org/#/c/22909/27/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/snmp/SnmpTrap.java File backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/snmp/SnmpTrap.java: Line 29: private static final Logger log = Logger.getLogger(SnmpTrap.class); Line 30: Line 31: public static final int ENTERPRISE_SPECIFIC = 6; Line 32: Line 33: private static Pattern SPLIT_PATTERN = Pattern.compile("(?<host>[^:\\s]+)(:(?<port>[^\\s]*))?"); you can reuse the same pattern, no? add a method of 'validate' to the transport to validate a configuration and call it from main? Line 34: Line 35: private NotificationProperties prop; Line 36: Line 37: private Snmp snmp; http://gerrit.ovirt.org/#/c/22909/27/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 87: public static final String SNMP_OID = "SNMP_OID"; Line 88: Line 89: private static final String GENERIC_MESSAGE = "Check configuration file, "; Line 90: Line 91: private static Pattern SPLIT_PATTERN = Pattern.compile("(?<host>[^:\\s]+)(:([^\\s]*))?"); I just would have aded <port> name to the port, so it will be easier to understand... also consider adding \\s* before and after. Line 92: Line 93: // Default files for defaults and overridden values: Line 94: private static String DEFAULTS_PATH = "/usr/share/ovirt-engine/conf/notifier.conf.defaults"; Line 95: private static String VARS_PATH = "/etc/ovirt-engine/notifier/notifier.conf"; Line 260: throw new IllegalArgumentException(GENERIC_MESSAGE + "illegal value for '" + SNMP_OID + "'"); Line 261: } Line 262: } Line 263: Line 264: private void validateSnmpAvailability() { is this working for all profiles? Line 265: Matcher m = SPLIT_PATTERN.matcher(getProperty(SNMP_MANAGERS)); Line 266: while(m.find()) { Line 267: validateHost(SNMP_MANAGERS, m.group("host")); Line 268: } -- 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: 27 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
