anmolbabu has posted comments on this change. Change subject: webadmin: add gluster37 support flag in cluster pop up ......................................................................
Patch Set 2: (4 comments) https://gerrit.ovirt.org/#/c/39757/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java: Line 992: } Line 993: if (getEnableGlusterService().getEntity() != null Line 994: && getEnableGlusterService().getEntity()) Line 995: { Line 996: getSupportGluster37Features().setIsAvailable(getVersion().getSelectedItem().equals(Version.v3_5)); Correct me if I am wrong but I feel, this must be in version change listener. Because, what if the user has enabled glusterservice and then the version was 3.5 itself but accidentally at creation time he chooses 3.4. I undersand in gluster-only mode the version as of now can't be changed(As user has to fist change data center version to change cluster version to lower value and datacenter is invisible in gluster-only mode. I remember seeing a bug for this sometime back) Line 997: getSupportGluster37Features().setIsChangable(!isEdit); Line 998: getEnableTrustedService().setEntity(false); Line 999: getEnableTrustedService().setIsChangable(false); Line 1000: } Line 993: if (getEnableGlusterService().getEntity() != null Line 994: && getEnableGlusterService().getEntity()) Line 995: { Line 996: getSupportGluster37Features().setIsAvailable(getVersion().getSelectedItem().equals(Version.v3_5)); Line 997: getSupportGluster37Features().setIsChangable(!isEdit); I assume the above step keeps the field changeable ONLY at new cluster creation time. But, if the nodes in the cluster already have the features in the rpms why blocking the edit? Or is it because, the nodes at deploy/bootstrap time look at this flag and download the rpms accordingly. Line 998: getEnableTrustedService().setEntity(false); Line 999: getEnableTrustedService().setIsChangable(false); Line 1000: } Line 1001: else Line 999: getEnableTrustedService().setIsChangable(false); Line 1000: } Line 1001: else Line 1002: { Line 1003: getSupportGluster37Features().setIsAvailable(false); Not really sure why we are doing it this way. I mean this way most(every) field would require 2 entries right? I feel getSupportGluster37Features().setIsAvailable(getEnableGlusterService().getEntity() != null && getEnableGlusterService().getEntity()) && getVersion().getSelectedItem().equa ls(Version.v3_5)); this avoids 2 entries right? Line 1004: if (getEnableOvirtService().getEntity() != null Line 1005: && getEnableOvirtService().getEntity()) Line 1006: { Line 1007: getEnableTrustedService().setIsChangable(true); Line 1627: setRngSourcesCheckboxes(version); Line 1628: Line 1629: updateFencingPolicyContent(version); Line 1630: Line 1631: if (getEnableGlusterService().getEntity() == Boolean.TRUE) { why not if (getEnableGlusterService().getEntity()) Line 1632: getSupportGluster37Features().setIsAvailable(getVersion().getSelectedItem().equals(Version.v3_5)); Line 1633: getSupportGluster37Features().setIsChangable(!getIsEdit()); Line 1634: } Line 1635: -- To view, visit https://gerrit.ovirt.org/39757 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic13dda67f3e9a7d7134030c3923470291c3c5aec Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: anmolbabu <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
