Alona Kaplan has posted comments on this change. Change subject: engine, webadmin: Webadmin read reports.xml from remote reports app ......................................................................
Patch Set 13: (6 comments) http://gerrit.ovirt.org/#/c/29723/13/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 59: // As this class has it's state, it needs to be inited again Line 60: initState(); Line 61: setReportBaseUrl((String) AsyncDataProvider.getInstance().getConfigValuePreConverted(ConfigurationValues.RedirectServletReportsPage)); Line 62: if (!reportBaseUrl.trim().isEmpty()) { Line 63: parseReportsXML(); Still looks unformatted. Maybe it is a problem with my view... Please run a formatter on the whole file. Line 64: } else { Line 65: setXmlInitialized(); Line 66: } Line 67: } Line 104: + BaseContextPathData.getInstance().getRelativePath() Line 105: + service; Line 106: } Line 107: Line 108: private void scheduleCheckStatus() { Why did you move the if from here? Should be- if (scheduledStatusCheckInProgress || retryCount > MAX_RETRY_COUNTS || reportsWebappDeployed) { return; } It is better to keep the if here over duplicating it to 'onError' and 'Response.SC_NOT_FOUND' Line 109: scheduledStatusCheckInProgress = true; Line 110: Scheduler.get().scheduleFixedDelay( Line 111: new Scheduler.RepeatingCommand() { Line 112: @Override Line 148: try { Line 149: constructServiceRequestBuilder(ReportXmlService).sendRequest(null, new RequestCallback() { //$NON-NLS-1$ Line 150: @Override Line 151: public void onError(Request request, Throwable exception) { Line 152: if (!reportBaseUrl.trim().isEmpty() && !scheduledStatusCheckInProgress && !reportsWebappDeployed) { 'if (!reportBaseUrl.trim().isEmpty())' is not needed here anymore- since it is part of the init. Line 153: scheduleCheckStatus(); Line 154: } Line 155: setXmlInitialized(); Line 156: } Line 158: @Override Line 159: public void onResponseReceived(Request request, Response response) { Line 160: switch (response.getStatusCode()) { Line 161: case Response.SC_NOT_FOUND: Line 162: if (!reportBaseUrl.trim().isEmpty() && !scheduledStatusCheckInProgress && !reportsWebappDeployed) { 'if (!reportBaseUrl.trim().isEmpty())' is not needed here anymore- since it is part of the init. Line 163: scheduleCheckStatus(); Line 164: } Line 165: setXmlInitialized(); Line 166: break; Line 170: resourceMap = ReportParser.getInstance().getResourceMap(); Line 171: dashboardMap = ReportParser.getInstance().getDashboardMap(); Line 172: isCommunityEdition = ReportParser.getInstance().isCommunityEdition(); Line 173: } Line 174: } catch (DOMParseException e) { Also looks unformatted for me. If it is formatted- ignore my formatting comments. Line 175: } finally { Line 176: setXmlInitialized(); Line 177: } Line 178: break; http://gerrit.ovirt.org/#/c/29723/13/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/CommonModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/CommonModel.java: Line 355: System Since it can take time to init the reports- what if the tree selected item is not system when they are initialized? The current treeSelectedItem should be passed, isn't it? -- 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: 13 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
