Yair Zaslavsky has posted comments on this change. Change subject: core: remove commons httpclient from provider proxy ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/33458/3/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 317: if (read > -1) { Line 318: bytesOs.write(chunk, 0, read); Line 319: } else { Line 320: break; Line 321: } > there something commons that does this loop, no? oh, you meant some inputStreamToOutputStream method? I will check. Line 322: } Line 323: result = bytesOs.toByteArray(); Line 324: handleResponseCode(connection.getResponseCode()); Line 325: } catch (SSLException e) { Line 320: break; Line 321: } Line 322: } Line 323: result = bytesOs.toByteArray(); Line 324: handleResponseCode(connection.getResponseCode()); > I think you should swap the following two lines, first get status, and if s ok Line 325: } catch (SSLException e) { Line 326: throw new VdcBLLException(VdcBllErrors.PROVIDER_SSL_FAILURE, e.getMessage()); Line 327: } catch (IOException e) { Line 328: handleException(e); Line 343: Line 344: URL hostUrl = getUrl(); Line 345: Connection connection = null; Line 346: try { Line 347: if (isSecured()) { > this should not be in this context, but in common place... all you need is This is the based on the logic of this specific class. Your suggestion exists in the Connection.java class. Line 348: if (hostUrl.getPort() == -1) { Line 349: hostUrl = Line 350: createURLWithDefaultPort( Line 351: hostUrl, http://gerrit.ovirt.org/#/c/33458/3/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssl/SecureConnector.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssl/SecureConnector.java: > can you please take the ProxyServletBase::createConnection as is? But what about the TrustStore? Line 1: package org.ovirt.engine.core.utils.ssl; Line 2: Line 3: Line 4: import java.net.URL; -- To view, visit http://gerrit.ovirt.org/33458 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I727d34c33f357b93560d4b5a1784b3733b7fa293 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [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
