Sahina Bose has posted comments on this change.
Change subject: engine: Manage Gluster hooks support
......................................................................
Patch Set 20: (21 inline comments)
Updated patchset to follow.
....................................................
File backend/manager/dbscripts/gluster_hooks_sp.sql
Line 27:
Line 28:
Line 29:
Line 30: Create or replace FUNCTION GetGlusterHookById(v_id UUID,
Line 31: v_returnAll
BOOLEAN=false)
v_returnAll is to indicate if all columns should be returned since content
column is heavy and not needed always. Would it help if the name of the
parameter is renamed?
Line 32: RETURNS SETOF gluster_hooks
Line 33: AS $procedure$
Line 34: BEGIN
Line 35: if (v_returnAll = true) then
Line 74: Create or replace FUNCTION GetGlusterHook(v_cluster_id UUID,
Line 75: v_gluster_command
VARCHAR(1000),
Line 76: v_stage VARCHAR(100),
Line 77: v_name VARCHAR(1000),
Line 78: v_returnAll BOOLEAN=false)
Same as above. rename v_returnAll to v_returnContent ?
Line 79: RETURNS SETOF gluster_hooks
Line 80: AS $procedure$
Line 81: BEGIN
Line 82: if (v_returnAll = true) then
Line 108:
Line 109: Create or replace FUNCTION DeleteGlusterHook(v_cluster_id UUID,
Line 110: v_gluster_command
VARCHAR(1000),
Line 111: v_stage VARCHAR(100),
Line 112: v_name VARCHAR(1000))
This deletes a single hook, so would rather call it DeleteGlusterHook
Line 113: RETURNS VOID
Line 114: AS $procedure$
Line 115: BEGIN
Line 116: DELETE FROM gluster_hooks
Line 119: END; $procedure$
Line 120: LANGUAGE plpgsql;
Line 121:
Line 122:
Line 123: Create or replace FUNCTION DeleteGlusterHooksByIds(v_ids
VARCHAR(5000))
Can do.
Line 124: RETURNS VOID
Line 125: AS $procedure$
Line 126: BEGIN
Line 127: DELETE FROM gluster_hooks
Line 171:
Line 172: Create or replace FUNCTION UpdateGlusterHook(v_id UUID,
Line 173: v_hook_status
VARCHAR(50),
Line 174: v_content_type
VARCHAR(50),
Line 175: v_checksum VARCHAR(256),
Good point, will add content as a parameter.
Line 176: v_conflict_status
INTEGER)
Line 177: RETURNS VOID
Line 178: AS $procedure$
Line 179: BEGIN
Line 276: RETURNS VOID
Line 277: AS $procedure$
Line 278: BEGIN
Line 279: DELETE FROM gluster_server_hooks
Line 280: WHERE hook_id = v_hook_id;
Yes. this is a good suggestion. Will add this.
Line 281: END; $procedure$
Line 282: LANGUAGE plpgsql;
Line 283:
Line 284: Create or replace FUNCTION DeleteGlusterServerHooksByIds(v_ids
VARCHAR(5000))
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterHooksQuery.java
Line 13: }
Line 14:
Line 15: @Override
Line 16: protected void executeQueryCommand() {
Line 17:
getQueryReturnValue().setReturnValue(getGlusterHookDao().getByClusterId(getParameters().getClusterId()));
No, if null is passed as clusterId, an empty list is returned.
Line 18: getQueryReturnValue().setSucceeded(true);
Line 19: }
Line 20:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterHookConflictFlags.java
Line 2:
Line 3: public enum GlusterHookConflictFlags {
Line 4: CONTENT_CONFLICT (1), //0b001
Line 5: STATUS_CONFLICT(2), //0b010
Line 6: MISSING_HOOK (4); //0b100
There would be multiple combination of conflicts, specifically 6 with the 3
conflict bits.
The enum is not a representation of the conflict value, but an indicator of
which bit is set to represent a conflict.
I feel with helper methods (in GlusterHookEntity) to read and set the conflict
flags, it is easier to work with integer than 7 text values.
Line 7:
Line 8: private Integer flag;
Line 9:
Line 10: private GlusterHookConflictFlags(Integer value) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterHookEntity.java
Line 38: private String content;
Line 39:
Line 40: private Integer conflictStatus;
Line 41:
Line 42: private List<GlusterServerHook> serverHooks;
Done
Line 43:
Line 44: @Override
Line 45: public Guid getId() {
Line 46: return getId(true);
Line 46: return getId(true);
Line 47: }
Line 48:
Line 49: public Guid getId(boolean generateIfNull) {
Line 50: if (id == null && generateIfNull) {
Will add a default constructor.
Line 51: id = Guid.NewGuid();
Line 52: }
Line 53: return id;
Line 54: }
Line 111: }
Line 112:
Line 113: public void setContentType(String contentType) {
Line 114: if (contentType != null) {
Line 115: this.contentType =
GlusterHookContentType.valueOf(contentType);
Don't understand. 'this' is needed here to avoid ambiguity.
Line 116: }
Line 117: }
Line 118:
Line 119: public String getChecksum() {
Line 182: && name.equals(hook.getName())
Line 183: && contentType == hook.getContentType()
Line 184: && conflictStatus == hook.getConflictStatus()
Line 185: && checksum.equals(hook.getChecksum())
Line 186: && status == hook.getStatus())) {
Will do.
Line 187: return false;
Line 188: }
Line 189: return true;
Line 190: }
Line 194: return getId();
Line 195: }
Line 196:
Line 197: public boolean isMissingHookConflict() {
Line 198: return (this.conflictStatus & MISSING_HOOK.getValue()) ==
MISSING_HOOK.getValue();
Ok.
Line 199: }
Line 200:
Line 201: public boolean isStatusConflict() {
Line 202: return (this.conflictStatus & STATUS_CONFLICT.getValue()) ==
STATUS_CONFLICT.getValue();
Line 205: public boolean isContentConflict() {
Line 206: return (this.conflictStatus & CONTENT_CONFLICT.getValue()) ==
CONTENT_CONFLICT.getValue();
Line 207: }
Line 208:
Line 209: public void setConflictValue(boolean missingHook, boolean
statusConflict, boolean contentConflict) {
This method sets the conflicStatus - does not consider existing value.
Will add other methods addStatusConflict, addContentConflict etc that changes
the existing values.
Line 210: this.conflictStatus = 0;
Line 211: if (missingHook) {
Line 212: this.conflictStatus = this.conflictStatus |
MISSING_HOOK.getValue();
Line 213: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterServerHook.java
Line 42: this.status = status;
Line 43: }
Line 44:
Line 45: public void setStatus(String status) {
Line 46: this.status = GlusterHookStatus.valueOf(status);
Done
Line 47: }
Line 48:
Line 49: public GlusterHookContentType getContentType() {
Line 50: return contentType;
Line 56:
Line 57: public void setContentType(String contentType) {
Line 58: if (contentType != null) {
Line 59: this.contentType =
GlusterHookContentType.valueOf(contentType);
Line 60: }
Will set the contentType to null - This would happen only in case of missing
hooks.
Line 61: }
Line 62:
Line 63: public String getChecksum() {
Line 64: return checksum;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterParameters.java
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7:
Line 8: /**
Line 9: * Parameter class with cluster Id as parameters. <br>
Line 10: * This will be used by gluster volume option help command and Gluster
Hooks query. <br>
True. changing
Line 11: */
Line 12: public class GlusterParameters extends VdcQueryParametersBase {
Line 13: private static final long serialVersionUID = -1224829720081853632L;
Line 14:
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java
Line 869: * Returns the singleton instance of {@link GlusterHooksDao}.
Line 870: *
Line 871: * @return the dao
Line 872: */
Line 873: public GlusterHooksDao getGlusterHookDao() {
Yes. will change
Line 874: return getDao(GlusterHooksDao.class);
Line 875: }
Line 876:
Line 877: public void setOnStartConnectionTimeout(int
onStartConnectionTimeout) {
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterHooksDaoTest.java
Line 100: hookIds.add(FixturesTool.HOOK_ID2);
Line 101: dao.removeAll(hookIds);
Line 102: List<GlusterHookEntity> hooks =
dao.getByClusterId(FixturesTool.CLUSTER_ID);
Line 103: assertNotNull(hooks);
Line 104: assertTrue(hooks.isEmpty());
Ok. Will add this
Line 105: }
Line 106:
Line 107: @Test
Line 108: public void testUpdateGlusterHookStatus() {
Line 113: }
Line 114:
Line 115: @Test
Line 116: public void testUpdateGlusterServerHookStatus() {
Line 117: dao.updateGlusterServerHookStatus(FixturesTool.HOOK_ID,
SERVER_ID, GlusterHookStatus.ENABLED);
Sounds good. will change.
Line 118: GlusterServerHook serverhook =
dao.getGlusterServerHook(FixturesTool.HOOK_ID, SERVER_ID);
Line 119: assertNotNull(serverhook);
Line 120: assertEquals(GlusterHookStatus.ENABLED,
serverhook.getStatus());
Line 121: }
Line 152: public void testUpdateGlusterHook() {
Line 153: dao.updateGlusterHook(getGlusterHook());
Line 154: GlusterHookEntity hook = dao.getById(FixturesTool.HOOK_ID);
Line 155: assertNotNull(hook);
Line 156: assertEquals(CHECKSUM, hook.getChecksum());
Done
Line 157: }
Line 158:
--
To view, visit http://gerrit.ovirt.org/10650
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If8b2d261ead720fccea6884acbc806ec79e52b36
Gerrit-PatchSet: 20
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Selvasundaram <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Selvasundaram <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
