feat(api): add /health endpoint to check workers #345
Conversation
sketch_map_tool/routes.py
Outdated
| result: list = celery_app.control.ping(timeout=1) | ||
| if result: | ||
| return {"status": "ok"} | ||
| return {"status": "fail"} |
There was a problem hiding this comment.
I think this still returns a 2xx status code. Is this on purpose?
There was a problem hiding this comment.
Its on purpose. I do not know of any good alternative. The endpoint did what it is supposed to do, check the celery worker and reports on it.
I think it is fine like this.
sketch_map_tool/routes.py
Outdated
| @@ -220,6 +220,15 @@ def download(uuid: str, type_: REQUEST_TYPES) -> Response: | |||
| return send_file(file, mimetype, download_name=download_name) | |||
|
|
|||
|
|
|||
| @app.route("/api/ping") | |||
There was a problem hiding this comment.
Is ping the term you want to use? I often saw health or maybe status as endpoint name. The term ping also refers to ICMP requests and not necessarily to TCP/QUIC requests.
There was a problem hiding this comment.
/status is already in use for querying the status of a certain task (e.g. map generation). /health I like but seems to encompass more than one ping to on part of the system. On the other hand, one can add new parts to the health endpoint over time.
There was a problem hiding this comment.
renamed to /health
matthiasschaub
requested review from
jlink and
joker234
and removed request for
jlink
matthiasschaub
force-pushed
the
healtcheck
branch
from
514db46
to
1ab3f15
Compare
joker234
changed the title
matthiasschaub
force-pushed
the
healtcheck
branch
from
f25573b
to
966025a
Compare
No description provided.