mirror of
https://github.com/healthchecks/healthchecks.git
synced 2026-04-25 23:15:49 +03:00
[GH-ISSUE #370] Should Checks List return UUID? #283
Labels
No labels
bug
bug
bug
feature
good-first-issue
new integration
pull-request
question
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/healthchecks#283
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @jameskirsop on GitHub (May 27, 2020).
Original GitHub issue: https://github.com/healthchecks/healthchecks/issues/370
I'm wondering if it makes sense that for both write and read-only API calls, that the
/api/v1/checksendpoint returns the UUID for each check.For my Grafana integration, I'd like to be able to query for all checks (using a read only account), and then allow the user to select a particular check. From that point forward it makes sense only query for the one check (to be more efficient), but currently the checks endpoint doesn't return the UUID so I can't store it for the purposes of querying specifically for it.
If we think this makes sense, I'll put together a PR.
@cuu508 commented on GitHub (May 27, 2020):
The UUID is left out on purpose: if you know check's UUID you can easily construct its ping URL. We want to be able to use the readonly API in public dashboards, where exposing ping URLs could cause problems.
To uniquely identify a check without exposing its ping URL there is the
unique_keyfield. We could look at adding aunique_keyquery parameter. There's already support for filtering by tag:Similarly, we could add:
@jameskirsop commented on GitHub (May 27, 2020):
That seems to make sense, though passing the unique key without having to use a query string would be cleaner, if it's possible without reinventing the wheel.
I'll have a think about some options and post back with some ideas unless someone comes up with one first.
@jameskirsop commented on GitHub (May 28, 2020):
The issue I've discovered with using
unique_key, is that we can't filter on it using a QuerySet.This makes it inefficient to use, because to filter on it we'd need to load all the Checks and do a list comprehension on the QuerySet to filter for the one with the matching key.
Given the
unique_keydoesn't change, I suggest it becomes a property that's saved in the database, so that we can filter for it.@cuu508 commented on GitHub (May 29, 2020):
@jameskirsop kudos on the completeness of the PR. You didn't cut any corners, which is great to see!
I have some performance concerns though:
The
unique_keycolumn in the PR doesn't have an index.GET SITE_ROOT/api/v1/checks/<unique_key>will require a sequential scan in the database. This would be a problem on bigger installations.Adding an index on the
unique_keycolumn could be problematic too: theapi_checktable gets updated on every ping. When a row is updated, the index pointing to it gets updated too. So adding one extra index has a performance cost for every single ping.On comparison,
selects checks by
project_idwhich is a foreign key, and so has an index already. Fetching, say, 1000 checks and filtering on the client side appears inelegant for sure. But it may in fact be the more performant solution:@cuu508 commented on GitHub (May 29, 2020):
Looking at it some more, in
api.views.get_check, we could changewith
which would then avoid a sequential scan.
@cuu508 commented on GitHub (May 29, 2020):
I like the
/api/v1/checks/<unique_key>route, compared to the extra query parameter which I had suggested (/v1/checks/?unique_key=...).I'm not convinced about adding an extra column as it requires a schema and data migration, and an overriden
Check.save()method.I think I would prefer if there would be two separate
get_checkviews – one for fetching by UUID (it would work same as currently) and a separate one for fetching byunique_key. This one would fetch checks byproject_idand then filter them byunique_keyin Python code.@jameskirsop commented on GitHub (Jun 11, 2020):
For this to work, we'd have to have
unique_keyas a database column.I don't mind the suggestion of having two different functions for the different views. My main issue is not being able to do a
get_object_or_404at the start of the function and having the code do more work than is otherwise/typically required to be able to check if the object exists (and even then, you wouldn't be able to use theget_object_or_404shortcut).Nonetheless, a new PR has been raised with changes that are in line with the desires of this ticket in the hope this can be merged to master soon.
@cuu508 commented on GitHub (Jun 11, 2020):
Great, thanks for the updated PR, merged!
I made one noteworthy change:
In urls.py, instead of routing both UUIDs and unique_keys to the same view, we can route unique_keys directly to the
get_check_unique(renamed toget_check_by_unique_key) view. This way:if type(code) = ...partPOST /api/v1/checks/<unique_key>orDELETE /api/v1/checks/<unique_key>.@cuu508 commented on GitHub (Jun 11, 2020):
Yeah, on the python side it doesn't look as elegant as get_object_or_404 for sure. But the alternative has drawbacks too:
I'd like to start with the naive approach, and revisit if performance starts to become a problem.