search

revsys / django-health-check

a pluggable app that runs a full check on the deployment, using a number of plugins to check e.g. database, queue server, celery processes, etc.
https://readthedocs.org/projects/django-health-check/
MIT License
1.22k stars 191 forks source link

Add ability to define and run a subset health checks #390

Closed panteparak closed 9 months ago

panteparak commented 10 months ago

This PR Introduces an ability to define and run a SUBSET of health checks.

It will be useful during certain period where it is only required to run specific health checks during a scenario.

It supports both via url /ht/<subset-name>/ and via Django command python manage.py health_check --subset <subset-name>

panteparak commented 10 months ago

Hi @codingjoe

Just a polite ping!.

Could I have some feedback on this PR.

Really love to see this feature in the official release of django-health-check.

panteparak commented 8 months ago

@frankwiles Just a friendly reminder: Don't forget to create a new release :)

saz commented 8 months ago

@frankwiles FYI: this is a breaking change, if you've got custom checks, as the signature of BaseHealthCheckBackend.check_status has changed.

TypeError: CeleryBeatCheckBackend.check_status() got an unexpected keyword argument 'subset'

I'll send a PR to fix the docs. Maybe it's possible to add some hint to the current release notes?

SpecLad commented 8 months ago

It's not clear to me what the point of the subset parameter even is. None of the builtin health check backends actually use it. Can't it be removed to restore compatibility?

ramwin commented 8 months ago

@frankwiles FYI: this is a breaking change, if you've got custom checks, as the signature of BaseHealthCheckBackend.check_status has changed.

TypeError: CeleryBeatCheckBackend.check_status() got an unexpected keyword argument 'subset'

I'll send a PR to fix the docs. Maybe it's possible to add some hint to the current release notes?

I agree with you, this merge request should be merged into 4.0.0 only.

saz commented 8 months ago

@SpecLad There's an example usage in the README (not within the docs).

SpecLad commented 8 months ago

@SpecLad There's an example usage in the README (not within the docs).

Where? The README hasn't even been updated to include the subset parameter.

saz commented 8 months ago

image Can't link to the proper section, but as said, it's only in the README, not the documentation.

SpecLad commented 8 months ago

This doesn't describe the subset parameter.

saz commented 8 months ago

Ah, sorry, misunderstanding on my side. It looks like as if the subset parameter on check_status isn't used anywhere.

@panteparak Can you shed some light on this?

llam15 commented 8 months ago

I would like to +1 that this is a breaking change and should not have been released as a minor version update. This caused all of our custom health checks to fail with a TypeError

...check_status() got an unexpected keyword argument 'subset'

The minor version upgrade from 3.17 to 3.18 was pulled automatically because we had specified our dependency django-health-check = "^3.16", and unexpectedly caused the health checks to stop working.

For others: we were able to fix our custom health checks by modifying the check_status definition as so:

def check_status(self, *args, **kwargs):
SpecLad commented 8 months ago

I made a PR to unbreak this: #414.