Greg Padgett has posted comments on this change.

Change subject: tools: notifier: filter messages based on severity
......................................................................


Patch Set 1:

(5 comments)

Thanks for the feedback, replies inline.

https://gerrit.ovirt.org/#/c/38442/1/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/filter/FirstMatchSimpleFilter.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/filter/FirstMatchSimpleFilter.java:

Line 22:             "\\s*" +
Line 23:             "((?<include>include)|(?<exclude>exclude))" +
Line 24:             ":" +
Line 25:             "((?<anymsg>\\*)|(?<message>\\w+))" +
Line 26:             "(?>" +
> what is '>' in this context?
Part of (?>X), for an independent, non-capturing group of X.  Works well here 
since the group will match the whole severity token (and its delimiter) or 
nothing at all.
Line 27:                 ":" +
Line 28:                 "((?<anyseverity>\\*)|(?<severity>\\w+))" +
Line 29:             ")?" +
Line 30:             "(?<recipient>" +


Line 24:             ":" +
Line 25:             "((?<anymsg>\\*)|(?<message>\\w+))" +
Line 26:             "(?>" +
Line 27:                 ":" +
Line 28:                 "((?<anyseverity>\\*)|(?<severity>\\w+))" +
> why not put here \\*|ERROR|WARNING|....
Nice, done.
Line 29:             ")?" +
Line 30:             "(?<recipient>" +
Line 31:                 "\\(" +
Line 32:                     "(" +


Line 102:             }
Line 103:         }
Line 104:     }
Line 105: 
Line 106:     private boolean severityMatches(
> why don't you hold the severity as AuditLogSeverity after pattern processin
Done (but retained the method to make processEvent() easier to follow)
Line 107:             String filterSeverityString,
Line 108:             AuditLogSeverity eventSeverity,
Line 109:             boolean isExclusion) {
Line 110:         // Severity matching works in the opposite way for inclusions 
and exclusions; a filter will


Line 138: 
Line 139:                 ret.add(
Line 140:                         new FilterEntry(
Line 141:                                 m.group("anymsg") != null ? null : 
m.group("message"),
Line 142:                                 m.group("anyseverity") != null ? null 
: m.group("severity"),
> if you put * in pattern of severity you can avoid anyseverity if you like.
Done
Line 143:                                 m.group("exclude") != null,
Line 144:                                 m.group("recipient") == null || 
m.group("anyrecipient") != null ? null : new Recipient(
Line 145:                                         m.group("transport"),
Line 146:                                         m.group("name")


Line 182:             }
Line 183: 
Line 184:             Recipient that = (Recipient) obj;
Line 185:             return (StringUtils.equals(this.transport, that.transport)
Line 186:                     && StringUtils.equals(this.name, that.name));
> not part of this patch I guess.
Thanks, mistakenly left that in, done.
Line 187:         }
Line 188: 
Line 189:         @Override
Line 190:         public int hashCode() {


-- 
To view, visit https://gerrit.ovirt.org/38442
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8abc34d56f1ede2fb51daf71dee293d08f198a3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Mooli Tayer <[email protected]>
Gerrit-Reviewer: [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