fix(dataplanes): show dataplane info even when /stats fails#4645
fix(dataplanes): show dataplane info even when /stats fails#4645johncowen wants to merge 1 commit intokumahq:masterfrom
Conversation
Signed-off-by: John Cowen <john.cowen@konghq.com>
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
schogges
left a comment
There was a problem hiding this comment.
Why did you move DataplaneRouteGuard and RootView? They were intentionally kept in legacy-data-planes because ones we can remove the legacy parts of data-planes we can also remove the route guard and root view. So everything that can be removed was kept in legacy-data-planes 🤔
|
Oh sorry should have said: It took me a while to figure out what the routing structure was here and I moved it a little to make it easier to grok. What I found out was that For example, we can't just remove the If we remove Its also a lot clearer to figure out what is going on if you can rule the Apart from this I moved the top most |
Hmm 🤔 I think the dependencies are correct. We are going to remove
For sure we can't "just remove"
No strong opinion on this 👍 |
|
I'm not super strong either tbf 😆 , although let me put an argument forward! Imagine if we were to feature flag Then of course we want to be able to turn the feature flag on and off depending on something (ok, granted we don't need to do this but bear with me!) If we had it like it was previous to this PR, if we turned off Whereas with this PR, the relationship is inverted, Granted once we no longer need/have Ideally of course we would be able to do these sorts of things from The Outside (which is what we do with our "boot-time" service container) then you don't need to have code anywhere on The Inside that deals with these sorts of usecases. (obvs I get we can't do that here because we are post-boot) I do thing the difference between both options is very small, but given the above and the fact that it took me a while to figure out where some Anyway, as I said not a strong opinion either, but there's my reasoning. This needs a rebase, gonna sort that and ping you for another review, lmk on the above, also happy to approve/merge and discuss further. Also fine to have a stronger opinion and I'd be fine to put it back if so 👍 |
I found a tiny loading/error bug whilst working on something else.
TLDR: There was a DataLoader waiting on
trafficbut not erroring ontraffics error. When I looked at it I don't think we should be using a DataLoader (neither a loader nor an error) and just wait for the data and show the rendered ui once its loaded. Errors are dealt with via error notifications elsewhere.I added a little test to prevent future regressions (and tested it breaks on master)
Previous to this PR, the
About Dataplanebox would show an eternal loader if this errored (relatively common) even though it could show all of the data about the Dataplane.