mooli tayer has posted comments on this change. Change subject: core: Foreman discovery host integration to ovirt ......................................................................
Patch Set 10: (6 comments) http://gerrit.ovirt.org/#/c/27621/10/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 87: ForemanDiscoveredHostWrapper fdw = Line 88: objectMapper.readValue(httpMethod.getResponseBody(), ForemanDiscoveredHostWrapper.class); Line 89: return mapDiscoveredHosts(Arrays.asList(fdw.getResults())); Line 90: } catch (Exception e) { Line 91: return null; Better return an empty list is such cases so you don't have to handle null in callers. use Collections.emptyList() Line 92: } Line 93: } Line 94: Line 95: private List<ExternalHostGroup> runHostGroupListMethod(HttpMethod httpMethod) { Line 98: ForemanHostGroupWrapper fhgw = Line 99: objectMapper.readValue(httpMethod.getResponseBody(), ForemanHostGroupWrapper.class); Line 100: return mapHostGroups(Arrays.asList(fhgw.getResults())); Line 101: } catch (Exception e) { Line 102: return null; Collections.emptyList() Line 103: } Line 104: } Line 105: Line 106: private List<ExternalOperatingSystem> runOperationSystemMethod(HttpMethod httpMethod) { Line 107: try{ Line 108: runHttpMethod(httpClient, httpMethod); Line 109: ForemanOperatingSystemWrapper fosw = Line 110: objectMapper.readValue(httpMethod.getResponseBody(), ForemanOperatingSystemWrapper.class); Line 111: return mapOperationSystem(Arrays.asList(fosw.getResults())); I think ForemanOperatingSystemWrapper.getResults() can be list. I'm pretty sure Jackson handles that well. Line 112: } catch (Exception e) { Line 113: return null; Line 114: } Line 115: } Line 219: Line 220: @Override Line 221: public List<ExternalComputeResource> getComputeResources() { Line 222: HttpMethod method = new GetMethod(COMPUTE_RESOURCES_HOSTS_ENTRY_POINT); Line 223: return runComputeResourceMethod(method); Here you will end up returning null if there is any exception. Better return an empty list is such cases so you don't have to handle null in callers. Collections.emptyList() Line 224: } Line 225: Line 226: private List<ExternalOperatingSystem> getOperationSystems() { Line 227: HttpMethod method = new GetMethod(OPERATION_SYSTEM_QUERY); http://gerrit.ovirt.org/#/c/27621/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostWrapper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostWrapper.java: Line 2: Line 3: import org.codehaus.jackson.annotate.JsonProperty; Line 4: Line 5: public class ForemanHostWrapper { Line 6: @JsonProperty("results") private ForemanHost[] results; annotation in its own line please Line 7: public ForemanHost[] getResults() { return results; } Line 8: public void setResults(ForemanHost[] hosts) { this.results = hosts; } http://gerrit.ovirt.org/#/c/27621/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanOperatingSystemWrapper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanOperatingSystemWrapper.java: Line 2: Line 3: import org.codehaus.jackson.annotate.JsonProperty; Line 4: Line 5: public class ForemanOperatingSystemWrapper { Line 6: @JsonProperty("results") private ForemanOperatingSystem[] results; can we have the annotation in it's own line Line 7: public ForemanOperatingSystem[] getResults() { return results; } Line 8: public void setResults(ForemanOperatingSystem[] operationsystem) { Line 9: this.results = operationsystem; Line 10: } -- 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: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
