Ravi Nori has posted comments on this change.

Change subject: engine, webadmin: Webadmin read reports.xml from remote reports 
app
......................................................................


Patch Set 12:

(6 comments)

http://gerrit.ovirt.org/#/c/29723/12/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 56:         // the re-init can happen after logout/login.
Line 57:         // As this class has it's state, it needs to be inited again
Line 58:         initState();
Line 59:         setReportBaseUrl((String) 
AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.RedirectServletReportsPage));
Line 60:         parseReportsXML();
> 1. please format
Done
Line 61:     }
Line 62: 
Line 63:     private void initState() {
Line 64:         reportsEnabled = false;


Line 143:                     + 
BaseContextPathData.getInstance().getRelativePath()
Line 144:                     + "services/reports-xml").sendRequest(null, new 
RequestCallback() { //$NON-NLS-1$
Line 145:                 @Override
Line 146:                 public void onError(Request request, Throwable 
exception) {
Line 147:                     if (!reportBaseUrl.trim().isEmpty()) {
> please format
Not sure if I am missing some formating here
Line 148:                         scheduleCheckStatus();
Line 149:                     }
Line 150:                     setXmlInitialized();
Line 151:                 }


Line 159:                                     scheduleCheckStatus();
Line 160:                                 }
Line 161:                                 break;
Line 162:                             case Response.SC_OK:
Line 163:                                 if 
(ReportParser.getInstance().parseReport(response.getText())) {
> Shouldn't the try-catch surround just this if?
The finally is for both do you prefer this?

case Response.SC_NOT_FOUND:
                                if (!reportBaseUrl.trim().isEmpty() && 
!scheduledStatusCheckInProgress && !reportsWebappDeployed) {
                                    scheduleCheckStatus();
                                }
                                setXmlInitialized();
                                break;
Line 164:                                     resourceMap = 
ReportParser.getInstance().getResourceMap();
Line 165:                                     dashboardMap = 
ReportParser.getInstance().getDashboardMap();
Line 166:                                     isCommunityEdition = 
ReportParser.getInstance().isCommunityEdition();
Line 167:                                 }


http://gerrit.ovirt.org/#/c/29723/12/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/MainTabReportsPresenter.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/MainTabReportsPresenter.java:

Line 70: 
Line 71:     private void checkUpdateReportsPanel(final String url,
Line 72:                                          final Map<String, 
List<String>> params) {
Line 73:         try {
Line 74:             
ReportInit.constructRequestBuilder(buildStatusUrl()).sendRequest(null, new 
RequestCallback() {
> If you"ll change constructServiceRequestBuilder as I suggested you can use 
Done
Line 75:                 @Override
Line 76:                 public void onError(Request request, Throwable 
exception) {
Line 77:                     
errorPopupManager.show(applicationConstants.reportsWebAppNotDeployedMsg());
Line 78:                 }


http://gerrit.ovirt.org/#/c/29723/12/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java:

Line 100:     protected void onLogin(final LoginModel loginModel) {
Line 101:         // Initialize reports
Line 102:         ReportInit.getInstance().init();
Line 103: 
Line 104:         // Perform login only after reports have been initialized
> Please update the comment.
Done
Line 105:         
ReportInit.getInstance().getReportsInitEvent().addListener(new IEventListener() 
{
Line 106:             @Override
Line 107:             public void eventRaised(Event ev, Object sender, 
EventArgs args) {
Line 108:                 updateAvailability();


Line 104:         // Perform login only after reports have been initialized
Line 105:         
ReportInit.getInstance().getReportsInitEvent().addListener(new IEventListener() 
{
Line 106:             @Override
Line 107:             public void eventRaised(Event ev, Object sender, 
EventArgs args) {
Line 108:                 updateAvailability();
> updateAvialability() calls -> updateAvailability(SystemTreeItemType.System,
Done
Line 109:             }
Line 110:         });
Line 111: 
Line 112:         performLogin(loginModel);


-- 
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: 12
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

Reply via email to