Martin Peřina has posted comments on this change.

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


Patch Set 18: Code-Review-1

(3 comments)

Until you fix MAIL_PORT range change check, you break patch 23129

http://gerrit.ovirt.org/#/c/22909/18/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 147:      */
Line 148:     public void validate() {
Line 149:         validateCommon();
Line 150: 
Line 151:         validateEmailBasic();
This should be called only if MAIL_SERVER is entered
Line 152:         if (isConfigured(NotificationProperties.MAIL_SERVER)) {
Line 153:             validateEmailAvailability();
Line 154:         }
Line 155:         validateSNMPBasic();


Line 191:     }
Line 192: 
Line 193:     private void validateEmailBasic() {
Line 194:         // validate MAIL_PORT
Line 195:         requireAll(MAIL_PORT);
Please add here mail port validation, you dropped this by mistake on rebase
Line 196: 
Line 197:         // validate MAIL_USER value
Line 198:         String emailUser = 
getProperty(NotificationProperties.MAIL_USER, true);
Line 199:         if (StringUtils.isEmpty(emailUser)


Line 324:     /**
Line 325:      * Returns {@code true} if mail transport encryption type is 
correctly specified,
Line 326:      * otherwise {@code false}
Line 327:      */
Line 328:     public boolean isSMTPEncryptionOptionValid() {
Please undo this renaming
Line 329:         return 
MAIL_SMTP_ENCRYPTION_NONE.equals(getProperty(MAIL_SMTP_ENCRYPTION, true))
Line 330:                 || 
MAIL_SMTP_ENCRYPTION_SSL.equals(getProperty(MAIL_SMTP_ENCRYPTION, true))
Line 331:                 || 
MAIL_SMTP_ENCRYPTION_TLS.equals(getProperty(MAIL_SMTP_ENCRYPTION, true));
Line 332:     }


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