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

Fix duplicated healthchecks - #135 #137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

holytshirt
Copy link

@holytshirt holytshirt commented Feb 7, 2023

  • AddDbContextCheck by default checks to see if it can connect to the database .CanConnectAsync() you can also run a custom query against the context
  • No longer need healthchecks per db provider
  • Name is not required by default it does nameof on the type now making it refactor friendly
-- IdentityServerConfigurationDbContext  context.Clients.AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [Clients] AS [c]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
-- IdentityServerPersistedGrantDbContext context.Keys.AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [Keys] AS [k]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
-- AdminIdentityDbContext context.Set<TUser>().AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [Users] AS [u]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
-- AdminLogDbContext context.Logs.AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [Log] AS [l]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
-- AdminAuditLogDbContext context.AuditLog.AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [AuditLog] AS [a]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
-- IdentityServerDataProtectionDbContext context.DataProtectionKeys.AnyAsync(token)
SELECT CASE
    WHEN EXISTS (
        SELECT 1
        FROM [DataProtectionKeys] AS [d]) THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END

- AddDbContextCheck by default checks to see if it can connect to the database `.CanConnectAsync()` you can also run a custom query against the context
- No longer need healthchecks per db provider
- Name is not required by default it does `nameof` on the type now making it refactor friendly
@holytshirt holytshirt marked this pull request as ready for review February 7, 2023 16:43
@holytshirt
Copy link
Author

holytshirt commented Feb 7, 2023

@skoruba
Using a query also means if there is no data in a table the healthcheck fails. Personally I would not have the queries and just make sure you can connect.

  1. You probably want to double check the queries, that you are happy with the tables they are querying.
  2. Check you are happy with me removing the name and moving it to nameof(dbcontext)
  3. Happy with using tags and the tag names, struggle with identityserver http check

@colin-freemarketfx
Copy link

Neat, that does look a lot tidier.

@skoruba
Copy link
Owner

skoruba commented Feb 7, 2023

@aiscrim - what do you think? thanks

@reydelleon-skuvault
Copy link

Did this ever come to anything? I have the same issue that this PR attempted to address. A few releases have been put out since this Pr was created but these changes were never included. Can the PR at least be closed to signify we should give up on it?

@skoruba
Copy link
Owner

skoruba commented Jul 7, 2024

Thank you for letting me know. I am going to revisit this PR.

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.

5 participants