mooli tayer has posted comments on this change. Change subject: core: Foreman discovery host integration to ovirt ......................................................................
Patch Set 4: (8 comments) http://gerrit.ovirt.org/#/c/27621/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanComputerResource.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanComputerResource.java: Line 1: package org.ovirt.engine.core.bll.host.provider.foreman; Line 2: Line 3: import java.io.Serializable; Line 4: Line 5: public class ForemanComputerResource implements Serializable { please reorder properties and then setters and getters Line 6: private static final long serialVersionUID = -3185315137494277207L; Line 7: Line 8: public String getName() { Line 9: return name; http://gerrit.ovirt.org/#/c/27621/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanDiscoveredHost.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanDiscoveredHost.java: Line 1: package org.ovirt.engine.core.bll.host.provider.foreman; Line 2: Line 3: import java.io.Serializable; Line 4: Line 5: public class ForemanDiscoveredHost implements Serializable { please reorder properties then getters and setters. Line 6: private static final long serialVersionUID = -6900772579678185173L; Line 7: Line 8: public String getMac() { Line 9: return mac; http://gerrit.ovirt.org/#/c/27621/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHost.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHost.java: Line 14: public void setIp(String ip) { Line 15: this.ip = ip; Line 16: } Line 17: Line 18: private String ip; properties on top then getters and setters Line 19: Line 20: public String getName() { Line 21: return name; Line 22: } http://gerrit.ovirt.org/#/c/27621/4/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 93: runHttpMethod(httpClient, httpMethod); Line 94: ForemanDiscoveredHostWrapper fdw = Line 95: objectMapper.readValue(httpMethod.getResponseBody(), ForemanDiscoveredHostWrapper.class); Line 96: return mapDiscoveredHosts(Arrays.asList(fdw.getResults())); Line 97: } catch (JsonParseException e) { I think here it is the same as doing only catch (IOException e) { handleException(e); } because the other two are IOExceptions two and you handle all the same. please apply to other blocks as well Line 98: handleException(e); Line 99: } catch (JsonMappingException e) { Line 100: handleException(e); Line 101: } catch (IOException e) { Line 261: int computeResourceId, Line 262: String mac, Line 263: String discoverName, Line 264: String rootPassword) { Line 265: final String uri = "{\n" + probably not a url. you can call this entityBody (an entity has headers and a body just like HttpMethods) Line 266: " \"discover\": {\n" + Line 267: " \"name\": \"" + vds.getName() + "\",\n" + Line 268: " \"hostgroup_id\": \"" + hg.getHostgroupId() + "\",\n" + Line 269: " \"environment_id\": \"" + hg.getEnvironmentId() + "\",\n" + Line 286: " }\n" + Line 287: " }\n" + Line 288: " }\n" + Line 289: "}"; Line 290: HttpMethod httpMethod = new PutMethod(DISCOVERED_HOSTS_ENTRY_POINT + "/" + discoverName); define as put and then you don't have to cast bellow Line 291: RequestEntity entity = new RequestEntity() { Line 292: @Override Line 293: public boolean isRepeatable() { Line 294: return false; Line 287: " }\n" + Line 288: " }\n" + Line 289: "}"; Line 290: HttpMethod httpMethod = new PutMethod(DISCOVERED_HOSTS_ENTRY_POINT + "/" + discoverName); Line 291: RequestEntity entity = new RequestEntity() { I think using a StringRequestEntity might be simpler. check about repeatability. Line 292: @Override Line 293: public boolean isRepeatable() { Line 294: return false; Line 295: } http://gerrit.ovirt.org/#/c/27621/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ExternalDiscoveredHost.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ExternalDiscoveredHost.java: Line 40: public void setLast_report(String last_report) { Line 41: this.last_report = last_report; Line 42: } Line 43: Line 44: private String last_report; In all of these data classes it is a lot better if you could adhere to java naming conversions(lastReport): try this: @JsonProperty(value = "last_report") private String lastReport; -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[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
