Oved Ourfali has posted comments on this change. Change subject: core: Foreman discovery host integration to ovirt ......................................................................
Patch Set 7: (3 comments) minor comments. http://gerrit.ovirt.org/#/c/27621/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostProviderProxy.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostProviderProxy.java: Line 45: Line 46: private HttpClient httpClient = new HttpClient(); Line 47: Line 48: private ObjectMapper objectMapper = new ObjectMapper(); Line 49: private static final String API_ENTRY_POINT = "/api/v2"; I wonder if we need also to still support v1. I don't think so, but will check it out. In the meantime, this can be merged. And, if we need v1 support then we'll have another class for that, and let the user select a version, and etc. But, out of the scope for now. Line 50: private static final String JSON_FORMAT = "format=json"; Line 51: Line 52: private static final String HOSTS_ENTRY_POINT = API_ENTRY_POINT + "/hosts"; Line 53: private static final String ALL_HOSTS_QUERY = HOSTS_ENTRY_POINT + "?" + JSON_FORMAT; http://gerrit.ovirt.org/#/c/27621/7/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 39: public boolean getAddProvisioned() { Line 40: return privateAddProvisioned; Line 41: } Line 42: Line 43: public void setAddProvisioned(Guid pid, ExternalHostGroup hg, int crId, String mac, String discover_name) { isn't it setting a boolean? if addProvisioned is a boolean, and getAddProvisioned returns boolean, then setting everything here? Please split it to different getters and setters. Line 44: hostMac = mac; Line 45: hostGroup = hg; Line 46: providerId = pid; Line 47: privateAddProvisioned = true; http://gerrit.ovirt.org/#/c/27621/7/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml File frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml: Line 321: <!-- Foreman Objects --> Line 322: <include name="common/businessentities/ExternalHostGroup.java"/> Line 323: <include name="common/businessentities/ExternalHost.java"/> Line 324: <include name="common/businessentities/ExternalDiscoveredHost.java"/> Line 325: <include name="common/businessentities/ExternalComputeResource.java"/> remove tabs. Line 326: </source> Line 327: Line 328: <super-source path="ui/uioverrides" /> -- 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: 7 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
