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 <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
