❌ DO NOT MERGE - [ADMINAPI-1260] Design: merge health check worker into AdminApi-2 #323

CSR2017 · 2025-08-04T18:08:58Z

No description provided.

github-actions · 2025-08-04T18:59:25Z

🔍 Vulnerabilities of `development:latest`

📦 Image Reference development:latest

digest	`sha256:b96184a7f1d66af83eefb311165abc1f16deca178fe569082c19fff92d08d21e`
vulnerabilities
platform	linux/amd64
size	122 MB
packages	635

📦 Base Image alpine:3

also known as	`3.20` `3.20.3` `latest`
digest	`sha256:33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735`
vulnerabilities

github-actions · 2025-08-04T19:01:08Z

Test Results

322 tests 321 ✅ 14s ⏱️
6 suites 1 💤
6 files 0 ❌

Results for commit c0e2e0d.

♻️ This comment has been updated with latest results.

docs/design/INTEGRATE-HEALTHCHECK-SERVICE

stephenfuqua · 2025-08-06T16:25:01Z

Docker/dev.pgsql.Dockerfile


+COPY --from=assets ./Application/NuGet.Config EdFi.AdminConsole.HealthCheckService/
+COPY --from=assets ./Application/EdFi.AdminConsole.HealthCheckService EdFi.AdminConsole.HealthCheckService/
+


These are frustrating false positives. I wish Hadolint would update to recognize the new syntax for additional contexts.

Application/Directory.Packages.props

docs/design/INTEGRATE-HEALTHCHECK-SERVICE

stephenfuqua · 2025-08-11T16:54:24Z

.github/workflows/api-e2e-mssql-multitenant.yml


+      - name: Copy health check service folder to docker context
+        run: cp -r ../EdFi.Ods.AdminApi.HealthCheck ../../Docker/Application
+


We could simplify all of this switching the Dockerfile to use the new "additional contexts" feature, which allows accessing files from a different parent directory.

Let's do that as a separate work item though, rather than allowing scope creep on this.

stephenfuqua · 2025-08-11T17:00:59Z

docs/docker.md

+
 ## Admin Api and Ed-Fi ODS / API docker containers

 Please refer [DOCKER DEPLOYMENT](https://techdocs.ed-fi.org/display/EDFITOOLS/Docker+Deployment) for


Suggested change

Please refer [DOCKER DEPLOYMENT](https://techdocs.ed-fi.org/display/EDFITOOLS/Docker+Deployment) for

Please refer [DOCKER DEPLOYMENT](https://docs.ed-fi.org/reference/docker) for

stephenfuqua · 2025-08-11T17:45:44Z

I moved to "ready for review" for Copilot's sake.

Copilot

Pull Request Overview

This PR integrates the Ed-Fi AdminConsole HealthCheckService directly into the EdFi.Ods.AdminApi application using Quartz.NET for scheduling and on-demand execution. The integration eliminates the need for a separate health check worker process by incorporating health check functionality as scheduled background jobs within the main AdminAPI application.

Key changes include:

Added a new EdFi.Ods.AdminApi.HealthCheck project with health check logic previously in a separate service
Integrated Quartz.NET for scheduled health check execution with configurable frequency
Added API endpoint for triggering health checks on-demand

Reviewed Changes

Copilot reviewed 52 out of 54 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
eng/setup-local-multi-tenants-docker.ps1	New PowerShell script for simplified multi-tenant Docker environment setup
docs/docker.md	Documentation for the new multi-tenant setup script
docs/design/INTEGRATE-HEALTHCHECK-SERVICE	Design document outlining the health check service integration architecture
Application/EdFi.Ods.AdminApi/Program.cs	Integration of health check services into main application startup
Application/EdFi.Ods.AdminApi.HealthCheck/*	New project containing health check service logic and API clients
Various appsettings files	Configuration updates for health check endpoints and multi-tenant setup
Docker files and CI workflows	Updates to include new health check project in build processes

Copilot · 2025-08-11T17:46:56Z

Application/EdFi.Ods.AdminApi.HealthCheck/EdFi.AdminConsole.HealthCheckService.nuspec

@@ -0,0 +1,17 @@
+?xml version="1.0"?>


Missing opening angle bracket. The XML declaration should be <?xml version="1.0"?>

Suggested change

?xml version="1.0"?>

<?xml version="1.0"?>

Copilot · 2025-08-11T17:46:56Z

Application/EdFi.Ods.AdminApi.HealthCheck/EdFi.AdminConsole.HealthCheckService.nuspec

+?xml version="1.0"?>
+<package >
+  <metadata>
+    <id><EdFi class="AdminConsoleHealthCheckWorkerProcess"></EdFi></id>


Invalid XML structure. The <id> element contains invalid nested tags. It should contain a plain string package identifier, not XML elements with attributes.

Suggested change

<id><EdFi class="AdminConsoleHealthCheckWorkerProcess"></EdFi></id>

<id>EdFi.AdminConsoleHealthCheckWorkerProcess</id>

Copilot · 2025-08-11T17:46:57Z

Application/EdFi.Ods.AdminApi.HealthCheck/Helpers/JsonBuilder.cs

+
+        if (healthCheckData != null)
+        {
+            healthCheckDocument.Add(new KeyValuePair<string, JsonNode?>("healthy", !healthCheckData.Any(r => r.AnyErrros)));


Typo in property name: AnyErrros should be AnyErrors

Suggested change

healthCheckDocument.Add(new KeyValuePair<string, JsonNode?>("healthy", !healthCheckData.Any(r => r.AnyErrros)));

healthCheckDocument.Add(new KeyValuePair<string, JsonNode?>("healthy", !healthCheckData.Any(r => r.AnyErrors)));

Copilot · 2025-08-11T17:46:57Z

Application/EdFi.Ods.AdminApi.HealthCheck/Features/OdsApi/OdsApiEndpointNameCount.cs

+
+    public int OdsApiEndpointCount { get; set; } = 0;
+
+    public bool AnyErrros { get; set; } = false;


Typo in property name: AnyErrros should be AnyErrors

Suggested change

public bool AnyErrros { get; set; } = false;

public bool AnyErrors { get; set; } = false;

Copilot · 2025-08-11T17:46:57Z

Application/EdFi.Ods.AdminApi.HealthCheck/Features/OdsApi/OdsApiCaller.cs

+        if (response != null && response.StatusCode == System.Net.HttpStatusCode.OK && response.Headers != null && response.Headers.Contains(Constants.TotalCountHeader))
+            result.OdsApiEndpointCount = int.Parse(response.Headers.GetValues(Constants.TotalCountHeader).First());
+        else
+            result.AnyErrros = true;


Typo in property name: AnyErrros should be AnyErrors

Suggested change

result.AnyErrros = true;

result.AnyErrors = true;

CSR2017 force-pushed the ADMINAPI-1260 branch from a6f33b8 to 8de3ca6 Compare August 4, 2025 18:58

stephenfuqua reviewed Aug 6, 2025

View reviewed changes

CSR2017 added 14 commits August 7, 2025 17:12

Add quartz.NET

1ccd9db

Update health check service

0aa1c43

Delete unwanted files

a90b19d

Update db ports

75cb247

Refactor the health check service files

36ba11a

Add health check endpoint

a3ff0c8

Add health check unit tests

34b70c0

Update dev docker .md file with powershell script details

7afb474

Add documentation for health check integration

a06e950

Fix merge conflicts

105ffe9

Fix build errors

f4c6f33

Add coverlet collector

465ab30

Add app http client unit tests

81d951a

Rename the project to EdFi.Ods.AdminApi.HealthCheck

c0e2e0d

CSR2017 force-pushed the ADMINAPI-1260 branch from 7a33b0e to c0e2e0d Compare August 7, 2025 22:12

stephenfuqua reviewed Aug 11, 2025

View reviewed changes

stephenfuqua marked this pull request as ready for review August 11, 2025 17:45

stephenfuqua requested a review from a team as a code owner August 11, 2025 17:45

stephenfuqua requested a review from Copilot August 11, 2025 17:45

Copilot AI reviewed Aug 11, 2025

View reviewed changes

CSR2017 marked this pull request as draft August 12, 2025 22:03

CSR2017 changed the title ~~[ADMINAPI-1260] Design: merge health check worker into AdminApi-2~~ ❌ DO NOT MERGE - [ADMINAPI-1260] Design: merge health check worker into AdminApi-2 Aug 12, 2025


		COPY --from=assets ./Application/NuGet.Config EdFi.AdminConsole.HealthCheckService/
		COPY --from=assets ./Application/EdFi.AdminConsole.HealthCheckService EdFi.AdminConsole.HealthCheckService/


		- name: Copy health check service folder to docker context
		run: cp -r ../EdFi.Ods.AdminApi.HealthCheck ../../Docker/Application


		## Admin Api and Ed-Fi ODS / API docker containers

		Please refer [DOCKER DEPLOYMENT](https://techdocs.ed-fi.org/display/EDFITOOLS/Docker+Deployment) for

	<id><EdFi class="AdminConsoleHealthCheckWorkerProcess"></EdFi></id>
	<id>EdFi.AdminConsoleHealthCheckWorkerProcess</id>

	healthCheckDocument.Add(new KeyValuePair<string, JsonNode?>("healthy", !healthCheckData.Any(r => r.AnyErrros)));
	healthCheckDocument.Add(new KeyValuePair<string, JsonNode?>("healthy", !healthCheckData.Any(r => r.AnyErrors)));


		public int OdsApiEndpointCount { get; set; } = 0;

		public bool AnyErrros { get; set; } = false;

	public bool AnyErrros { get; set; } = false;
	public bool AnyErrors { get; set; } = false;

❌ DO NOT MERGE - [ADMINAPI-1260] Design: merge health check worker into AdminApi-2 #323

Are you sure you want to change the base?

❌ DO NOT MERGE - [ADMINAPI-1260] Design: merge health check worker into AdminApi-2 #323

Uh oh!

Conversation

CSR2017 commented Aug 4, 2025

Uh oh!

github-actions bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Vulnerabilities of development:latest

Uh oh!

github-actions bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Uh oh!

stephenfuqua Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stephenfuqua Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

stephenfuqua Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

stephenfuqua commented Aug 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Aug 4, 2025 •

edited

Loading

🔍 Vulnerabilities of `development:latest`

github-actions bot commented Aug 4, 2025 •

edited

Loading