Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STORM-2344 Flux YAML File Viewer for Nimbus UI #1922

Merged
merged 1 commit into from
Feb 10, 2017
Merged

STORM-2344 Flux YAML File Viewer for Nimbus UI #1922

merged 1 commit into from
Feb 10, 2017

Conversation

ambud
Copy link
Contributor

@ambud ambud commented Feb 4, 2017

Adding capabilities to visualize the Storm Flux YAML files before submitting the topology would be helpful for developers and operations.

@@ -0,0 +1,139 @@
<html>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing apache license. And I guess we may need a link from storm ui homepage to flux.html.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vesense I thought someone will ask for that :D

Just added both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we plan on back porting this to 0.10.x and 1.x?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @vesense means that Storm UI index page should have a link to go to flux page. You're adding reverse link, which is already missing.

And I think we might want to port back to 1.x but not necessary to 1.0.x. Let's keep the bugfix version line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I misunderstood. @HeartSaVioR not sure what location in the homepage do we want to put the forward link?

One option could be to skip the link in homepage and provide it via Ambari View.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same idea as @HeartSaVioR 😄

Copy link
Contributor Author

@ambud ambud Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a link on the top right hand side after the username place holder. Here's the icon https://github.com/ambud/storm/blob/af8ed339cb3896100dabe3619c0fba2d19165dff/storm-core/src/ui/public/images/flux.png

Y for YAML and also for Flux Capacitor from Back to the future 💯

Please let me know if this works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure which is better between F (for Flux) and Y (for Yaml). I guess the question could be changed to is flux yaml viewer flux yaml viewer, or flux yaml viewer. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And posting screenshot to the comment would help us to see.

Copy link
Contributor Author

@ambud ambud Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storm-ui

Just rendering it using a simple web server

@HeartSaVioR
Copy link
Contributor

@ambud
We need to update LICENSE file to reflect newly added js files.
https://github.com/apache/storm/blob/master/LICENSE

Could you update it?

@HeartSaVioR
Copy link
Contributor

And some javascript files like esprima.js are not minimized. Is there any reason not to do?

Adding apache license and link to Storm Homepage

Adding links from storm nimbus homepage

Adding License for Javascript libraries. Using min js for esprima

Adding license files
@ambud
Copy link
Contributor Author

ambud commented Feb 9, 2017

  • Added license
  • Squashed commits.

Couldn't find a min.js for dagre

@HeartSaVioR
Copy link
Contributor

js files are licensed under BSD-2 and MIT, seems OK.
Regarding min.js, I think you're saying about cytoscape-dagre.js, which is under 200 lines so it would be OK.

+1 from me. Thanks!

@asfgit asfgit merged commit d6c3145 into apache:master Feb 10, 2017
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.

4 participants