Centralize check registration / running logic #85

grahamalama · 2023-08-10T18:47:31Z

This PR adds to the dockerflow.checks package to provide the ability to register checks, run registered checks (both synchronously and asynchronously), and return the results of the checks in a standardized manner.

Since Django already has the concept of a check registry, we just run the checks there (but with our check runner). For other frameworks, we use the new registry in dockerflow.checks.registry.

Still todo is ~~add tests for the check running functions~~ (to make assertions about the shape of the data they return) and update documentation. But I'm open to review in this PR's current state.

codecov-commenter · 2023-08-10T18:51:05Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (a7cad34) 97.84% compared to head (880630f) 98.23%.

Files	Patch %	Lines
src/dockerflow/checks/registry.py	96.29%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   97.84%   98.23%   +0.39%     
==========================================
  Files          18       19       +1     
  Lines         602      624      +22     
  Branches       88       85       -3     
==========================================
+ Hits          589      613      +24     
+ Misses          9        7       -2     
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- remove version check - autouse so that checks are cleared after every test run

leplatrem

👌 Top!

src/dockerflow/checks/registry.py

leplatrem · 2023-08-18T08:28:48Z

src/dockerflow/checks/registry.py

+    Adds a given check callback with the provided object to the list
+    of checks. Useful for built-ins but also advanced custom checks.
+    """
+    logger.debug("Adding extension check %s" % check.__name__)


I see that this comes from the Django implementation, but I don't really understand the vocabulary here: why is it an extension check?

Is it specific to Django built-in checks or are we going to use this in other integrations? Maybe it should remain under dockerflow.django

These are great questions. I had ones that were similar. The init_check / "extension check" vocabulary existed before this PR -- it was a pattern for Flask and Sanic. I don't actually see any traces of it in Django 🤔.

It does seem like it could be reworked. I'll give it a try in this PR, but if things get too crazy, maybe it deserves its own.

Oops you're right, misread the diff at the moment of writing the comment :)
Don't hesitate to open a follow-up if you think that's too much for this PR

src/dockerflow/sanic/checks.py

leplatrem · 2023-08-18T09:22:32Z

src/dockerflow/checks/registry.py

+def init_check(check, obj):
+    """
+    Adds a given check callback with the provided object to the list
+    of checks. Useful for built-ins but also advanced custom checks.


Why init check?
Could we add types and/or docstrings for the input parameters for better clarity? Or provide an example on how to use this init_check()?

grahamalama · 2023-08-18T13:49:45Z

src/dockerflow/checks/registry.py

+    Adds a given check callback with the provided object to the list
+    of checks. Useful for built-ins but also advanced custom checks.
+    """
+    logger.debug("Adding extension check %s" % check.__name__)


These are great questions. I had ones that were similar. The init_check / "extension check" vocabulary existed before this PR -- it was a pattern for Flask and Sanic. I don't actually see any traces of it in Django 🤔.

It does seem like it could be reworked. I'll give it a try in this PR, but if things get too crazy, maybe it deserves its own.

src/dockerflow/sanic/checks.py

src/dockerflow/flask/app.py

leplatrem · 2023-09-22T07:44:19Z

@grahamalama shall we resume this work?

…istry

This reverts commit 80ef729.

leplatrem · 2023-12-14T16:35:07Z

@grahamalama r?

grahamalama · 2023-12-14T17:41:21Z

Nice @leplatrem, thanks for taking this over and finishing it 🙌🏻

But oh no, I can't actually approve this PR because I was an author on some of the commits 😱.

You could pull another reviewer in, or you can consider this my approval and bypass branch protections.

* Add tox config for FastAPI 100 * Add dockerflow router to package root * Add request summary middleware * Add version endpoint and tests * Add initial heartbeat route, check mechanism, and tests * Refactor package structure, names of things - Define "router" in package init, register view functions defined in views module - Add test for check with custom name - use better name for check operations * Leverage centralized checks (#85) * Do not test on Python 3.7 * Add 'taskName' to excluded fields * Test with python 3.12 * Improve test coverage of FastAPI * Blackify * FastAPI integration docs (#95) * Add FastAPI documentation * Update docs/fastapi.rst Co-authored-by: grahamalama <graham.beckley@gmail.com> * Mention APP_DIR for the version file --------- Co-authored-by: grahamalama <graham.beckley@gmail.com> * Update tox.ini Co-authored-by: grahamalama <graham.beckley@gmail.com> --------- Co-authored-by: Mathieu Leplatre <mathieu@mozilla.com>

dockerflow(object).init_check -> dockerflow(module).checks.register_partial dockerflow(object).check -> dockerflow(module).checks.register See mozilla-services/python-dockerflow#85

grahamalama added 3 commits August 10, 2023 14:28

Add central check registry / runner

0cc0fd1

Add pytest fixture to automatically clear check registry

c75fc6f

Use centralized check registry in Flask

1c16991

grahamalama requested a review from leplatrem August 10, 2023 18:47

grahamalama added 6 commits August 10, 2023 15:04

Use centralized check registry in Sanic

8366b90

Use correct path for django test urlconf

8c6a4fc

Delete Django level_to_text helper

7b938dc

Delete commented-out default Django check

658befa

Refactor reset_checks django fixture

1b6919e

- remove version check - autouse so that checks are cleared after every test run

Use centralized check runner in Django

fdc0148

grahamalama force-pushed the centralized-check-registry branch from 9cb8198 to fdc0148 Compare August 10, 2023 19:04

Tests for running checks

fa9fb5f

grahamalama force-pushed the centralized-check-registry branch from 1d0e0f9 to c5da374 Compare August 15, 2023 23:02

Add flask tests for registering migration check with app

7776fcf

grahamalama force-pushed the centralized-check-registry branch from c5da374 to 7776fcf Compare August 15, 2023 23:08

grahamalama marked this pull request as ready for review August 15, 2023 23:12

leplatrem reviewed Aug 18, 2023

View reviewed changes

grahamalama commented Aug 18, 2023

View reviewed changes

leplatrem added 8 commits November 15, 2023 15:33

Merge remote-tracking branch 'origin/main' into centralized-check-reg…

454914e

…istry

Adjust wording in docstring and logs

e17054b

Remove superfluous return

b70ba80

Restore legacy methods with deprecation warning

06909df

Rename init_check to register_partial

1c48eb7

Merge remote-tracking branch 'origin/main' into centralized-check-reg…

3d378a5

…istry

Fix posargs passing to pytest

dc2318c

Remove problematic test

80ef729

leplatrem force-pushed the centralized-check-registry branch from 3386818 to 80ef729 Compare December 13, 2023 16:12

Revert "Remove problematic test"

15eb348

This reverts commit 80ef729.

leplatrem added 3 commits December 14, 2023 16:29

Use a fixture to add logger handlers for request.summary

1e78930

Reset logging before loading ini

880630f

Run black 24.1a1

1f06328

leplatrem approved these changes Dec 15, 2023

View reviewed changes

leplatrem merged commit 886ac45 into main Dec 15, 2023

leplatrem deleted the centralized-check-registry branch December 15, 2023 08:57

leplatrem added a commit that referenced this pull request Feb 14, 2024

Leverage centralized checks (#85)

717ec5b

Eijebong mentioned this pull request Aug 27, 2025

Fix dockerflow related deprecation warnings mozilla-releng/shipit#1780

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Centralize check registration / running logic #85

Centralize check registration / running logic #85

Uh oh!

grahamalama commented Aug 10, 2023 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 10, 2023 •

edited

Loading

Uh oh!

leplatrem left a comment

Uh oh!

Uh oh!

Uh oh!

leplatrem Aug 18, 2023

Uh oh!

grahamalama Aug 18, 2023 •

edited

Loading

Uh oh!

leplatrem Aug 22, 2023

Uh oh!

Uh oh!

leplatrem Aug 18, 2023

Uh oh!

grahamalama Aug 18, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

leplatrem commented Sep 22, 2023

Uh oh!

leplatrem commented Dec 14, 2023

Uh oh!

grahamalama commented Dec 14, 2023

Uh oh!

Uh oh!

Centralize check registration / running logic #85

Centralize check registration / running logic #85

Uh oh!

Conversation

grahamalama commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

leplatrem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

leplatrem Aug 18, 2023

Choose a reason for hiding this comment

Uh oh!

grahamalama Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leplatrem Aug 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leplatrem Aug 18, 2023

Choose a reason for hiding this comment

Uh oh!

grahamalama Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

leplatrem commented Sep 22, 2023

Uh oh!

leplatrem commented Dec 14, 2023

Uh oh!

grahamalama commented Dec 14, 2023

Uh oh!

Uh oh!

grahamalama commented Aug 10, 2023 •

edited

Loading

codecov-commenter commented Aug 10, 2023 •

edited

Loading

grahamalama Aug 18, 2023 •

edited

Loading

grahamalama Aug 18, 2023 •

edited

Loading