-
Notifications
You must be signed in to change notification settings - Fork 142
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
Events Tab implementation #81
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! That is very cool!
I think we can reuse the approach of requesting the table for the pods as well. It's less data and the only way I know to get pod ages without timing sync issue with the client.
.get({ generator: update_events_table.bind({ events_table, debug, highlightEvent })}) | ||
cancellations.add(key, cancellation); | ||
return promise; | ||
}).catch(error => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think error should be caught in the fail
method of the spinner otherwise it keeps spinning.
top : '50%', | ||
bottom : '1', | ||
width : '100%', | ||
height : '50%-1', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the status bar at the bottom is not displayed.
I've tried to reproduce scaling deployments up and down without any luck. I'm wondering if this is specific to kamel-k that creates events but doesn't populate some standard fields (like namespace). Out of curiosity, could you run |
I think this raises an important case in that some fields aren't necessarily populated. As this will also be relevant in other Kubernetes Resources, I thought it would be okay to add a check in the |
Awesome. Did you get a change to look at the status bar not displaying? Otherwise I think we can merge the PR and have a look afterwards. |
Note to myself, check how sorting is implemented in:
|
Let me resolve the conflicts and merge the PR! |
For sure! I'm still working on handling |
@astefanutti resolved merge conflicts, I also pushed hanlding |
@johnpoth Thanks a lot! Let's merge the PR and iterate. Once it's polished enough, I'll cut a release with all the good stuff that has been done since 0.7.0. |
This is a first stab at #36 to support events.
Hopefully we'll be able to reuse most of this for other resource types (via inheritance or composition).
We could also display a list of resource names similar to what we do for namespaces to support things like custom resources.
Once we agree on the display I'll polish things up a bit :)
HTH !
Note: I had to modify
lib/ui/blessed/element.js
for performance reasons.