Martin Peřina has posted comments on this change. Change subject: restapi: Host Power-Management Refactor (#977674) - WIP ......................................................................
Patch Set 34: (1 comment) http://gerrit.ovirt.org/#/c/27578/34/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java: Line 16: Line 17: private static final long serialVersionUID = -5910560758520427911L; Line 18: private Guid id; Line 19: private Guid hostId; Line 20: private Integer order; > 1) Turns out all engine entities must provide an empty constructor, so I ca > 1) Turns out all engine entities must provide an empty constructor, so I > can't do what > I said (provide only a constructor that gets 'order'). But since clients > (API, GUI) > already validate that order is set, I agree to wave this extra later of > protection, so > I will make FenceAgent-->order an int. If you set default order to 1 here, you provided: 1. Reasonable default for most cases 2. Client may always overwrite it So IMHO the best solution is to provide default noparam constructor for FenceAgent, where you can set default order to 1. > 2) I'm kind of on the fence about the need to enforce numbering like you > suggested. > It's obvious that smaller number is run first, and that agents with the same > number > always run concurrently. What do I care if, for example, two agent are > ordered 1 and 3? > We'd have to add code that renumbers the agents, and more code equals more > possible > bugs. I don't mind discussing this some more and giving it more thought, but > in any case > I think this change will be left to a subsequent patch. 1. IMHO it's most natural if all agents has the same ordering values (primary 1, secondary 2, ...) 2. In webadmin UI you shouldn't show order at all, just display agents by it's order and before saving edited agents set proper order value 3. In REST API just reject request which has improper numbering And once again, if we all agree to this solution, we should provided it inside this patch, because it introduces order attribute and not to push one incomplete solution and fix it later. Eli, Oved, thoughts? Line 21: Line 22: @EditableField Line 23: private HashMap<String, String> optionsMap; Line 24: -- To view, visit http://gerrit.ovirt.org/27578 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0b384347a52db8b0aa6ae684fad40a5491dd2f7 Gerrit-PatchSet: 34 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
