-
Notifications
You must be signed in to change notification settings - Fork 451
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
Dashboard for Toxiproxy #218
base: main
Are you sure you want to change the base?
Conversation
This is awesome! I've wanted a dashboard built-in for a while, but we haven't really had a completed one until now. Here's a couple of initial feedback items:
@jpittis @sirupsen Have you got any thoughts on this? I'd love to have this merged. |
Thanks, @xthexder for your feedback. Regarding your feedback points,
|
👍 for root path I haven't had to update the vendoring on this project recently, so my knowledge is a little rusty too. It's a code dependency as well, so it will need to be vendored anyway at some point. If you do it, then great, otherwise I could update it after merging. I think for now the current js/css setup is fine, if we want it work offline, that can be done in a separate PR. I think the same can be said about a toxic types endpoint. |
Hi @xthexder |
Thanks for this contribution! It looks like there's some security issues around Toxiproxy access from a browser, so we're going to have to sort those out before this gets merged. It shouldn't be too hard to fix, but I think we've actually got some issues even in the current release that need to be fixed first. |
@xthexder were those security issues sorted out so that this can be merged? |
@dianadevasia By renaming (I think using the rename feature of GitHub would have prevented this.) |
UI might need more refining