Alona Kaplan has posted comments on this change. Change subject: engine, webadmin: Webadmin read reports.xml from remote reports app ......................................................................
Patch Set 10: (3 comments) http://gerrit.ovirt.org/#/c/29723/10/backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/ReportsXmlServlet.java File backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/ReportsXmlServlet.java: Line 46: copy > I tried IOUtils.copy first, it did not work. I had to copy the code from IO Strange. Maybe you can debug to understand what is the problem. It is really ugly to copy this code... http://gerrit.ovirt.org/#/c/29723/10/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportInit.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportInit.java: Line 108: Scheduler.get().scheduleFixedDelay( Line 109: new Scheduler.RepeatingCommand() { Line 110: @Override Line 111: public boolean execute() { Line 112: if (retryCount > MAX_RETRY_COUNTS || reportsWebappDeployed) { > Not required to call setXmlInitialized here, it is called in parseReportsXM Since you've added 'if (isReportsEnabled() && !initEventRaised)' to 'checkIfInitFinished()' redundant calls to 'setXmlInitialized()' are harmful. But still, just in sake of the correctness of the code- 'setXmlInitialized()' should be called just after the xml is initialized or not initialized and there are no more attempts to initialize it. That means it should be called- 1. from here if (retryCount > MAX_RETRY_COUNTS). 2. from parseReportsXML()->onResponseReceived(..)->Response.SC_OK->finally (the try catch should surround just the Response.SC_OK case). 3. Within the catch surrounding parseReportsXML(). Line 113: scheduledStatusCheckInProgress = false; Line 114: return false; Line 115: } Line 116: retryCount++; Line 147: } Line 148: } Line 149: Line 150: private void parseReportsXML() { Line 151: RequestBuilder requestBuilder = > I dont see a point of creating a new method for creating a RequestBuilder b The purpose is to avoid duplication. I meant the new method will look something like this- public static RequestBuilder constructServiceRequestBuilder(String service) { return new RequestBuilder(RequestBuilder.GET, "/"+ BaseContextPathData.getInstance().getRelativePath() + "services/" + service); } The services- reports-status and reports-xml should be constants (public static final). Line 152: new RequestBuilder(RequestBuilder.GET, Line 153: "/" //$NON-NLS-1$ Line 154: + BaseContextPathData.getInstance().getRelativePath() Line 155: + "services/reports-xml"); //$NON-NLS-1$ -- To view, visit http://gerrit.ovirt.org/29723 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76db7ab889f21de083bb3c8276e8abb77b68fdb3 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Shirly Radco <[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
