Alon Bar-Lev has posted comments on this change.

Change subject: tools: notifier - simple first match include/exclude.
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.ovirt.org/#/c/24018/8/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 220: 
Line 221:     private void validateFilter() {
Line 222:         String filter = getProperty(NotificationProperties.FILTER, 
true);
Line 223:         if (filter != null)
Line 224:             FirstMatchSimpleFilter.parse(filter);
> correct, this way I suggest to do this in own patch and stack it before thi
as it does not make any sense to me... I dag some more...

NotificationProperties is LocalConfig which stores all properties within... so 
by inheritance the NotificationProperties does hold data.

It is called from main:

        try {
            prop = NotificationProperties.getInstance();
            prop.validate();
        } catch ...

There is no reason why you cannot hold digested variables which are digested 
values within this class, similar to its parent LocalConfig.

Alternatively, if you find such reason that I can understand, you can do this 
easily in the main...

        try {
            prop = NotificationProperties.getInstance();
            prop.validate();
            rules = FirstMatchSimpleFilter.parse(filter);
        } catch ...

Or you can add a method 'load' before validate, the 'load' may also through 
validation exceptions.

        try {
            prop = NotificationProperties.getInstance();
            prop.load();
            prop.validate();
        } catch ...

You can also substitute load with parse if that's makes it more clear for you.
Line 225:     }
Line 226: 
Line 227:     private void requireAll(String... mandatoryProperties) {
Line 228:         for (String property : mandatoryProperties) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0b94ee8665ff8030b36b463207e50beea44b47d
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@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

Reply via email to