Allon Mureinik has posted comments on this change. Change subject: engine: Refresh gluster data periodically ......................................................................
Patch Set 20: (2 inline comments) patchset 20: answered questions inline .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java Line 72: private static final String ENTITY_OPTION = "option"; Line 73: private static GlusterManager instance = new GlusterManager(); Line 74: private static Log log = LogFactory.getLog(GlusterManager.class); Line 75: private static final int GLUSTER_REFRESH_RATE_LIGHT = Line 76: Config.<Integer> GetValue(ConfigValues.GlusterRefreshRateLight); You're initializing a static variable with Config.GetValue. This means that during the class loading, Config's implementation (which, by default, goes to the database) will be called. This makes the class a bit hard to test - i.e., you have a sort of a race between the classloading of MockConfigRule and this class. If MockConfigRule is called first, you're fine. If not - you have a problem. If you replace this variable with a method, you will no longer have to worry about calling database-related code during class loading. private int getGlusterRefreshRateLight() { return Config.<Integer> GetValue(ConfigValues.GlusterRefreshRateLight); } Moreover, in the current implementation, you're disabling the ability to change this config value in runtime - it will be loaded and stored once. Line 77: private static final int GLUSTER_REFRESH_RATE_HEAVY = Line 78: Config.<Integer> GetValue(ConfigValues.GlusterRefreshRateHeavy); Line 79: Line 80: private GlusterManager() { .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/NonOperationalReason.java Line 13: VERSION_INCOMPATIBLE_WITH_CLUSTER(5), Line 14: KVM_NOT_RUNNING(6), Line 15: TIMEOUT_RECOVERING_FROM_CRASH(7), Line 16: VM_NETWORK_IS_BRIDGELESS(8), Line 17: GLUSTER_COMMAND_FAILED(9), Missed that, good point. Thanks for sharing. Line 18: ; Line 19: Line 20: private final int value; Line 21: -- To view, visit http://gerrit.ovirt.org/7288 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b61eb6e93105e46e2706eac1d94bc10717224c2 Gerrit-PatchSet: 20 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches