Yair Zaslavsky has posted comments on this change. Change subject: core: remove commons httpclient from provider proxy ......................................................................
Patch Set 12: (6 comments) http://gerrit.ovirt.org/#/c/33458/12/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 274: " }\n" + Line 275: " ]\n" + Line 276: " }\n" + Line 277: "}"; Line 278: runHttpModificatioMethod(HttpMethodType.PUT, Paths.get(DISCOVERED_HOSTS_ENTRY_POINT,discoverName).toString(), entityBody); > ","->", "? not sure i understand what you mean here. Line 279: } Line 280: Line 281: @Override Line 282: public void testConnection() { Line 301: Line 302: private void runHttpModificatioMethod(HttpMethodType httpMethod, String relativeUrl, String body) { Line 303: HttpURLConnection connection = createConnection(relativeUrl); Line 304: try { Line 305: connection.setRequestProperty("Content-Type", "application/json"); > you need to add content encoding with utf8 Done Line 306: connection.setDoOutput(true); Line 307: connection.setRequestMethod(httpMethod.toString()); Line 308: if (body != null) { Line 309: byte[] bytes = body.getBytes(Charset.forName("UTF-8")); Line 376: private void handleCredentials(HttpURLConnection connection) { Line 377: if (hostProvider.getUsername() != null && hostProvider.getPassword() != null) { Line 378: connection.setRequestProperty( Line 379: "Auhtorization", Line 380: new StringBuilder("Basic ").append( > string.format? Done Line 381: Base64.encodeBase64( Line 382: new StringBuilder( Line 383: hostProvider.getUsername()) Line 384: .append(":") http://gerrit.ovirt.org/#/c/33458/12/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java: Line 23 Line 24 Line 25 Line 26 Line 27 > I do not think this is required either. Done Line 69 Line 70 Line 71 Line 72 Line 73 > again, please remind me why this is required. see usages - by git grep. Dont we need this at getCertificateChain? and at ForemanHostProviderProxy - we should check wheter to set all SSL related stuff. Why perform the check based on the protocol twice in code, and not have this help method? Line 77 Line 78 Line 79 Line 80 Line 81 > this is not required as far as I can guess. Done -- 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: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@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
