Yaniv Bronhaim has posted comments on this change. Change subject: core: Foreman discovery host integration to ovirt ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/27621/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java: Line 86: // check if a name is updated to an existing vds name Line 87: else if (!StringUtils.equalsIgnoreCase(_oldVds.getName(), getParameters().getVdsStaticData() Line 88: .getName()) Line 89: && getVdsDAO().getByName(vdsName) != null Line 90: && getVdsDAO().getByName(vdsName).getStatus() != VDSStatus.InstallingOS) { > 1.) probably wrong to go to the db twice You are right. i don't recall why i left this logic here. probably mistake. Line 91: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); Line 92: } else if (!StringUtils.equalsIgnoreCase(_oldVds.getHostName(), getParameters().getVdsStaticData() Line 93: .getHostName()) Line 94: && getVdsDAO().getAllForHostname(hostName).size() != 0 Line 91: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); Line 92: } else if (!StringUtils.equalsIgnoreCase(_oldVds.getHostName(), getParameters().getVdsStaticData() Line 93: .getHostName()) Line 94: && getVdsDAO().getAllForHostname(hostName).size() != 0 Line 95: && getVdsDAO().getByName(vdsName).getStatus() != VDSStatus.InstallingOS) { also here.. Line 96: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST); Line 97: } else if (getParameters().getInstallVds() && _oldVds.getStatus() != VDSStatus.Maintenance Line 98: && _oldVds.getStatus() != VDSStatus.NonOperational Line 99: && _oldVds.getStatus() != VDSStatus.InstallFailed http://gerrit.ovirt.org/#/c/27621/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostGroup.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostGroup.java: Line 7: private static final long serialVersionUID = -3099054972843803212L; Line 8: Line 9: private String name; Line 10: private int id; Line 11: private int subnet_id; > use java naming convention? it must be same as appears in the json response Line 12: private int operatingsystem_id; Line 13: private int domain_id; Line 14: private int environment_id; Line 15: private int ptable_id; http://gerrit.ovirt.org/#/c/27621/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVdsActionParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVdsActionParameters.java: Line 62: public boolean isGlusterPeerProbeNeeded() { Line 63: return this.glusterPeerProbeNeeded; Line 64: } Line 65: Line 66: public boolean getAddPending() { > In this file boolean getters are names isX and getX. that's how it was before my change. I don't see a problem with that. so i added getAddProvisioned as getAddPending was. think its great Line 67: return privateAddPending; Line 68: } Line 69: Line 70: public void setAddPending(boolean value) { -- To view, visit http://gerrit.ovirt.org/27621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c25a544373d8ab4a7123137c8a4697205a8efa8 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> 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
