Piotr Kliczewski has posted comments on this change. Change subject: jsonrpc: Stomp changes in vdsbroker ......................................................................
Patch Set 2: (4 comments) http://gerrit.ovirt.org/#/c/26783/2/backend/manager/dependencies/src/main/modules/org/apache/qpid-proton-j/main/module.xml File backend/manager/dependencies/src/main/modules/org/apache/qpid-proton-j/main/module.xml: Line 5: <resources> Line 6: <resource-root path="proton-j.jar"/> Line 7: </resources> Line 8: Line 9: <!-- dependencies> > please remove comments if not needed Done Line 10: <module name="javax.api"/> Line 11: <module name="org.apache.commons.logging"/> Line 12: <module name="org.codehaus.jackson.jackson-mapper-asl"/> Line 13: <module name="org.codehaus.jackson.jackson-core-asl"/> http://gerrit.ovirt.org/#/c/26783/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java: Line 310: model.setAddress(entity.getHostName()); Line 311: if (entity.getPort() > 0) { Line 312: model.setPort(entity.getPort()); Line 313: } Line 314: model.setProtocol(entity.getProtocol()); > Hmm, now that i think about it, why protocol is not an enum? I change that to provide enum. Line 315: HostStatus status = map(entity.getStatus(), null); Line 316: model.setStatus(StatusUtils.create(status)); Line 317: if (status==HostStatus.NON_OPERATIONAL) { Line 318: model.getStatus().setDetail(entity.getNonOperationalReason().name().toLowerCase()); http://gerrit.ovirt.org/#/c/26783/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java: Line 188: int clientTimeOut = Config.<Integer> getValue(ConfigValues.vdsTimeout) * 1000; Line 189: int connectionTimeOut = Config.<Integer> getValue(ConfigValues.vdsConnectionTimeout) * 1000; Line 190: int clientRetries = Config.<Integer> getValue(ConfigValues.vdsRetries); Line 191: Line 192: if (_vds.getProtocol() == 1) { > well, of course I can read the code and understand, but - still, maybe enum Agree. Will provide enum Line 193: _vdsProxy = new JsonRpcVdsServer(JsonRpcUtils.createStompClient(_vds.getHostName(), Line 194: _vds.getPort(), connectionTimeOut, Line 195: clientTimeOut, clientRetries, Line 196: Config.<Boolean> getValue(ConfigValues.EncryptHostCommunication))); http://gerrit.ovirt.org/#/c/26783/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/JsonRpcUtils.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/JsonRpcUtils.java: Line 16: Line 17: public class JsonRpcUtils { Line 18: private static Log log = LogFactory.getLog(JsonRpcUtils.class); Line 19: Line 20: public static JsonRpcClient createTcpClient(String hostname, int port, int connectionTimeout, > please order methods in such a way that public static methods come before p Done Line 21: int clientTimeout, int connectionRetry, boolean isSecure) { Line 22: return createClient(hostname, port, connectionTimeout, clientTimeout, connectionRetry, isSecure, ReactorType.TCP); Line 23: } Line 24: -- To view, visit http://gerrit.ovirt.org/26783 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If78de6620ba6891543531ac8ddd633b67828a89c Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
