Alon Bar-Lev has posted comments on this change. Change subject: engine, webadmin: Webadmin read reports.xml from remote reports app ......................................................................
Patch Set 19: (9 comments) http://gerrit.ovirt.org/#/c/29723/19/backend/manager/modules/services/src/main/webapp/WEB-INF/web.xml File backend/manager/modules/services/src/main/webapp/WEB-INF/web.xml: Line 56: </servlet-mapping> Line 57: Line 58: <servlet> Line 59: <servlet-name>reports-redirect</servlet-name> Line 60: <servlet-class>org.ovirt.engine.core.utils.servlet.RedirectServlet</servlet-class> so remove ReportsRedirectServlet? Line 61: <init-param> Line 62: <param-name>url</param-name> Line 63: <param-value>%{ENGINE_REPORTS_BASE_URL}</param-value> Line 64: </init-param> Line 89: </init-param> Line 90: </servlet> Line 91: <servlet-mapping> Line 92: <servlet-name>reports-servlet</servlet-name> Line 93: <url-pattern>/reports-servlet</url-pattern> servlet is not good term... you expose technical term... which is unrelated. reports-proxy reports-channel Line 94: </servlet-mapping> Line 95: Line 96: <servlet> Line 97: <servlet-name>health</servlet-name> http://gerrit.ovirt.org/#/c/29723/19/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 28: private static final ReportInit INSTANCE = new ReportInit(); Line 29: private static final int MAX_RETRY_COUNTS = 20; Line 30: private static final int RETRY_INTERVAL = 30000; Line 31: public static final String ReportStatusService = "services/reports-servlet?command=status"; //$NON-NLS-1$ Line 32: public static final String ReportXmlService = "services/reports-servlet?command=xml"; //$NON-NLS-1$ command=webadmin-ui-xml? I also recommend to have command=version to receive version and see if you are compatible... or add header at response with version. Line 33: private int retryCount; Line 34: private boolean reportsWebappDeployed; Line 35: private boolean scheduledStatusCheckInProgress; Line 36: private boolean reportsEnabled; Line 157: } Line 158: Line 159: @Override Line 160: public void onResponseReceived(Request request, Response response) { Line 161: switch (response.getStatusCode()) { please always have default statement Line 162: case Response.SC_INTERNAL_SERVER_ERROR: Line 163: scheduleCheckStatus(); Line 164: setXmlInitialized(); Line 165: break; http://gerrit.ovirt.org/#/c/29723/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java: Line 964: if (!isAllCanDoPassed) { Line 965: break; Line 966: } Line 967: } Line 968: if (isAllCanDoPassed) { please avoid format changes Line 969: cancel(); Line 970: } Line 971: } Line 972: }, null); http://gerrit.ovirt.org/#/c/29723/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java: Line 520: cancel(); Line 521: Line 522: } Line 523: }, windowModel); Line 524: } else { please avoid format changes Line 525: cancel(); Line 526: } Line 527: } Line 528: }), http://gerrit.ovirt.org/#/c/29723/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java: please remove format changes. http://gerrit.ovirt.org/#/c/29723/19/packaging/services/ovirt-engine/ovirt-engine.conf.in File packaging/services/ovirt-engine/ovirt-engine.conf.in: Line 213 Line 214 Line 215 Line 216 Line 217 this is not required now Line 214: # Reports Line 215: # by default serv 404. Line 216: # Line 217: ENGINE_REPORTS_BASE_URL=/ovirt-engine-reports Line 218: ENGINE_REPORTS_SERVLET_URL=${ENGINE_REPORTS_BASE_URL}/ReportsServlet this must be full url... no? default: ENGINE_REPORTS_COMMAND_INTERFACE_URL="http://localhost:${ENGINE_PROXY_HTTP_PORT}/ovirt-engine-reports/..."; or maybe you want this disabled per default, and enable only if reports is installed? or maybe you want the base url above as full url? Line 219: ENGINE_REPORTS_UI=${ENGINE_VAR}/reports.xml Line 220: Line 221: -- 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: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Shirly Radco <sra...@redhat.com> Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
