Yair Zaslavsky has posted comments on this change. Change subject: uutils: Extract connection class ......................................................................
Patch Set 4: (2 comments) http://gerrit.ovirt.org/#/c/33479/4/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/http/Connection.java File backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/http/Connection.java: > please rename to HttpURLConnectionFactory Not sure if HttpURLConnectionFactory is the correct name. You obtain a Connection object, which allows you to do some method chanining. In addition, as this is in the utils.http package, I find the word Http to be redundant in the class name. Line 1: package org.ovirt.engine.core.uutils.http; Line 2: Line 3: import java.io.FileInputStream; Line 4: import java.io.IOException; Line 146: public HttpURLConnection getURLConnection() { Line 147: if (!(connection instanceof HttpsURLConnection)) { Line 148: throw new RuntimeException("The connection is not an HTTP or HTTPS connection"); Line 149: } Line 150: return (HttpURLConnection) connection; > why do you need t o store this as member? This is the inner member Connection wraps. getURLConnection only returns the member, it does not create it. I would prefer for convenience of usage to have such a method. Notice also that create() returns Connection and not HttpUrlConnection to support some method chaining, so we have to provide somehow the HttpUrlConnection, to allow setRequestProperty and other methods that were not introduced to Connection per your previous comments. Line 151: } Line 152: Line 153: -- To view, visit http://gerrit.ovirt.org/33479 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I85ea4e7301b3a018b0438fff25cefad80ebd7256 Gerrit-PatchSet: 4 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
