Skip to content

Conversation

@caofangkun
Copy link
Contributor

➜  storm-core git:(storm-668) mvn clean &&  mvn install -Dmaven.test.skip=true -Dwar
➜  storm-core git:(storm-668) cp target/storm-ui.war $TOMCAT_HOME/webapps/

Merge from apache/storm to caofangkun/apache-storm
Merge from apache/storm to caofangkun/apache-storm
 Merge from apache/storm to caofangkun/apache-storm
@d2r
Copy link

d2r commented Jun 15, 2015

@caofangkun This is an interesting idea. With the provided code, we can run storm ui to verify things are working as expected. Testing this functionality is going to be more difficult, as it would require spinning up a tomcat server to verify everything still works. Do you have advice on how we might be able to do that?

@caofangkun
Copy link
Contributor Author

@d2r
Thank you for your attention of this PR.
In my opinion ,
1, "storm ui" is more like a RESTful APIs server than a UI server.
2, We can use jetty embedded server (as the provided code use) for testing;
but on production environment, use Apache Tomcat or other WebSever Container may be a better choice.
I have implement these as a external project storm-ui .

@d2r
Copy link

d2r commented Jun 17, 2015

@caofangkun OK, let's add a short section to DEVELOPER.md or another doc with:

  • Build and tomcat integration instructions from your earlier comment
  • Disclaimer saying that it is not tested and so it may not work correctly

It will not be built by default, but only if we include -Dwar, so I think it should be OK.

@revans2
Copy link
Contributor

revans2 commented Jan 5, 2016

@caofangkun it looks like there were only a few minor nits before this could go in. Do you still want this to go in? if so could you please upmerge and update the documentation as @d2r has requested?

@caofangkun
Copy link
Contributor Author

@revans2 This branch is outdated too much, I pushed a new request as #992 .

@caofangkun caofangkun closed this Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants