Alexander Wels has posted comments on this change.

Change subject: services: add proxy servlet
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/30740/4/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/servlet/ProxyServletBase.java
File 
backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/servlet/ProxyServletBase.java:

Line 183:                 }
Line 184:             } catch(Exception e) {
Line 185:                 throw new ServletException(e);
Line 186:             }
Line 187:         }
> this is what this function is doing... extra abstraction does not contribut
In my eyes the doGet is the main processing loop,  the point of the processing 
is to act like a proxy. As part of the processing it needs to create a 
connection to the proxied url. How this connection happens or what kind of 
connection is really not that important for the main processing loop. So to me 
it makes sense to the creation of the connection to a separate method.

If you made the method protected, then a sub class of this could in theory 
apply their own connection creation if they wanted.
Line 188:         connection.connect();
Line 189:         try {
Line 190:             if (connection instanceof HttpURLConnection) {
Line 191:                 
response.setStatus(((HttpURLConnection)connection).getResponseCode());


-- 
To view, visit http://gerrit.ovirt.org/30740
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia26768ec7a0e06a20f657cae40f6a51303970b44
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yaniv Dary <[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