-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: update make dev user ui ping url #850
fix: update make dev user ui ping url #850
Conversation
03a7290
to
0496611
Compare
tools/dev.sh
Outdated
@@ -118,7 +118,7 @@ user_ui_pid=$! | |||
|
|||
( | |||
for _ in {1..60}; do # ~1 minute timeout | |||
if curl -s --head http://localhost:8080/ | head -n 1 | grep "200 OK" > /dev/null; then | |||
if curl -s -H 'User-Agent: Mozilla/5.0' --head http://localhost:8080/unknown | head -n 1 | grep "200 OK" > /dev/null; then |
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.
Just checked and you don't actually need the header for this to work, since we have -s --head
specified.
Let's also switch this to check for the .ico file served by the user UI instead since I think it's probably a bad idea to depend on /<agent>
returning a 200 when you try to GET an agent that doesn't exist; I suspect that behavior is actually a bug, we probably want some splash page there that tells you that an admin needs to configure some agents.
if curl -s -H 'User-Agent: Mozilla/5.0' --head http://localhost:8080/unknown | head -n 1 | grep "200 OK" > /dev/null; then | |
if curl -s --head http://localhost:8080/favicon.ico | head -n 1 | grep "200 OK" > /dev/null; then |
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.
You also want to drop the http://localhost:8080/admin/
from the open
command a few lines below
I checked and this url is actually still valid. It doesn't look like admin is served from |
Signed-off-by: Ryan Hopper-Lowe <[email protected]>
- remove duplicate header flag
6b48387
to
953a2cf
Compare
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 checked and this url is actually still valid. It doesn't look like admin is served from localhost:8080/ yet
Your browser should be redirected from http://localhost:8080
to http://localhost:8080/admin/
automatically, so it doesn't make sense to open both IMO.
Ah I think I see what you're saying now. Updated |
Signed-off-by: Ryan Hopper-Lowe [email protected]