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

Wait for Plausible active state #3153

Merged
merged 6 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion frontend/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ up *flags:

# Wait for all profile services to be up
wait-up: up
echo "🚧 TODO"
just wait
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it'd be fine to move the contents of the wait recipe here and remove the @wait recipe below entirely!

Copy link
Member

Choose a reason for hiding this comment

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

I think this is implemented this way for consistency with the API justfile which has both @wait and wait-up. I can get behind this, as this can make working in a Dockerised context locally easier in the future.


# Set up user and test site in Plausible
init: wait-up
Expand All @@ -71,3 +71,19 @@ run *args:

types:
cd .. && pnpm exec vue-tsc -p frontend --noEmit

##########
# Health #
##########

# Check the health of the service
@health host="localhost:50288":
python3 plausible_health_check.py {{host}}

# Wait for the service to be healthy
@wait host="localhost:50288":
# The just command on the second line is executed in the context of the
# parent directory and so must be prefixed with `api/`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# parent directory and so must be prefixed with `api/`.
# parent directory and so must be prefixed with `frontend/`.

just ../_loop \
'"$(just frontend/health {{ host }})" != "true"' \
"Waiting for the frontend to be healthy..."
Copy link
Member

Choose a reason for hiding this comment

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

Along the same lines as @sarayourfriend's feedback, this should be updated as follows.

Suggested change
"Waiting for the frontend to be healthy..."
"Waiting for Plausible to be healthy..."

22 changes: 22 additions & 0 deletions frontend/plausible_health_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import sys

import requests
Copy link
Collaborator

Choose a reason for hiding this comment

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

requests isn't standard library and not everyone will have it installed in their global python installation (I don't, for example). urlopen is a better choice here, but less convenient, if we stick with a python script instead of a shell-based approach.

+1 to Dhruv's comment that try/except is just fine here, totally appropriate usage.



def main():
try:
host = sys.argv[1]
url = f"http://{host}/api/health/"
resp = requests.get(url=url)

information = resp.json()
for data in information:
if information[data] != "ok":
return sys.stdout.write("false")
return sys.stdout.write("true")
except ConnectionError:
pass


if __name__ == "__main__":
main()
Loading