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

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

Merged
merged 17 commits into from
Dec 13, 2023

Conversation

panteparak
Copy link
Contributor

@panteparak panteparak commented Nov 25, 2023

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 panteparak changed the title Add ability to only run certain health checks Add ability to define and run a subset health checks Nov 25, 2023
@panteparak
Copy link
Contributor Author

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.

@codingjoe codingjoe requested a review from frankwiles December 11, 2023 09:13
Copy link
Member

@frankwiles frankwiles left a comment

Choose a reason for hiding this comment

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

This looks great @panteparak I think it's definitely worth including. I'm hoping to do another release in the next few days (was actually supposed to do it last weekend but something came up) so I can include this in there.

@frankwiles frankwiles merged commit 4da1fd9 into revsys:master Dec 13, 2023
23 checks passed
@panteparak
Copy link
Contributor Author

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

@saz
Copy link
Contributor

saz commented Jan 29, 2024

@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?

saz added a commit to saz/django-health-check that referenced this pull request Jan 29, 2024
@SpecLad
Copy link
Contributor

SpecLad commented Jan 29, 2024

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
Copy link

ramwin commented Jan 30, 2024

@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
Copy link
Contributor

saz commented Jan 30, 2024

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

@SpecLad
Copy link
Contributor

SpecLad commented Jan 30, 2024

@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
Copy link
Contributor

saz commented Jan 30, 2024

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

@SpecLad
Copy link
Contributor

SpecLad commented Jan 30, 2024

This doesn't describe the subset parameter.

@saz
Copy link
Contributor

saz commented Jan 30, 2024

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
Copy link

llam15 commented Jan 30, 2024

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
Copy link
Contributor

SpecLad commented Jan 30, 2024

I made a PR to unbreak this: #414.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants