Piotr Kliczewski has posted comments on this change.

Change subject: events: VM Status based on an event
......................................................................


Patch Set 24:

(2 comments)

https://gerrit.ovirt.org/#/c/37488/24/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RefresherFactory.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RefresherFactory.java:

Line 16:         Version version = manager.getCompatibilityVersion();
Line 17:         if (FeatureSupported.jsonProtocol(version) && 
VdsProtocol.STOMP.equals(manager.getCopyVds().getProtocol())
Line 18:                 && FeatureSupported.vmStatsEvents(version)) {
Line 19:             return new EventVMStatsRefresher(manager, 
auditLogDirector, scheduler);
Line 20:         }
> s/VdsProtocol.STOMP.equals(/VdsProtocol.STOMP ==
Done
Line 21:         return new PollVMStatsRefresher(manager, auditLogDirector, 
scheduler);
Line 22:     }


https://gerrit.ovirt.org/#/c/37488/24/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/EventVMStatsRefresher.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/EventVMStatsRefresher.java:

Line 29: import org.reactivestreams.Subscription;
Line 30: import org.slf4j.Logger;
Line 31: import org.slf4j.LoggerFactory;
Line 32: 
Line 33: public class EventVMStatsRefresher extends PollVMStatsRefresher {
> I think this kind of inheritance is confusing since EventVMStatsRefresher i
The idea was to introduce abstraction without having separate interface or 
abstract class. Common stuff is in poll in refresher if we introduce new 
abstraction all the code will be gone from poll refresher.

If this is confusing the rename should be better to have it as VMStatsRefresher.
Line 34:     private static final Logger log = 
LoggerFactory.getLogger(EventVMStatsRefresher.class);
Line 35:     private Subscription subscription;
Line 36:     private DbFacade dbFacade;
Line 37:     private ResourceManager resourceManager;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b35877ccd63372759ad6989280e9417c259b21
Gerrit-PatchSet: 24
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to