Yair Zaslavsky has posted comments on this change.

Change subject: uutils: Extract connectionBuilder class
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.ovirt.org/#/c/33479/5/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/http/ConnectionBuilder.java
File 
backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/http/ConnectionBuilder.java:

Line 32:     private String url;
Line 33:     private URLConnection connection;
Line 34: 
Line 35:     public ConnectionBuilder() {
Line 36:     }
> please delete the extra space-line
Done
Line 37: 
Line 38: 
Line 39:     public ConnectionBuilder(URL url) {
Line 40:         this(url.toString());


Line 90:     }
Line 91: 
Line 92: 
Line 93:     public HttpURLConnection create() throws IOException, 
GeneralSecurityException {
Line 94:         connection = new URL(url).openConnection();
> connection should be a local variable, and create() produces a new instance
Done
Line 95:         connection.setAllowUserInteraction(false);
Line 96:         connection.setUseCaches(false);
Line 97:         if (readTimeout != null) {
Line 98:             connection.setReadTimeout(readTimeout);


Line 143:             }
Line 144:         }
Line 145:         if (!(connection instanceof HttpsURLConnection)) {
Line 146:             throw new RuntimeException("The connection is not an HTTP 
or HTTPS connection");
Line 147:         }
> please check the protocol first within the URL.... no need to reach to this
Done
Line 148:         return (HttpURLConnection) connection;
Line 149:     }
Line 150: 
Line 151:     public ConnectionBuilder setURLWithRelativePath(URL url, String 
relativePath) throws MalformedURLException {


Line 147:         }
Line 148:         return (HttpURLConnection) connection;
Line 149:     }
Line 150: 
Line 151:     public ConnectionBuilder setURLWithRelativePath(URL url, String 
relativePath) throws MalformedURLException {
> move this above along with other setters, rename to appendRelativePath?
Done
Line 152:         int port = url.getPort() == -1 ? url.getDefaultPort() : 
url.getPort();
Line 153:         this.url = new URL(url.getProtocol(), url.getHost(), port, 
new File(url.getFile(), relativePath).toString()).toString();
Line 154:         return this;
Line 155:     }


-- 
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: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[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

Reply via email to