Skip to content

fix: rename currentRole to current-role in health check response#3291

Open
Copilot wants to merge 4 commits intomainfrom
copilot/rename-current-role-to-current-role
Open

fix: rename currentRole to current-role in health check response#3291
Copilot wants to merge 4 commits intomainfrom
copilot/rename-current-role-to-current-role

Conversation

Copy link
Contributor

Copilot AI commented Mar 18, 2026

Why make this change?

The /health endpoint response used currentRole (camelCase) while every other multi-word key in the response uses kebab-case (app-name, cache-ttl-seconds, response-ms, etc.), creating an inconsistent API surface.

What is this change?

  • Updated [JsonPropertyName("currentRole")][JsonPropertyName("current-role")] in ComprehensiveHealthCheckReport.cs.
  • Added retry logic (up to 10 attempts, 1s apart) to HotReloadConfigConnectionString and HotReloadConfigDatabaseType integration tests to handle a pre-existing race condition: the "Validated hot-reloaded configuration file" success message is logged before metadata re-initialization completes, causing intermittent REST request failures. This is consistent with the same fix already applied to HotReloadConfigDataSource.

Before:

{
  "status": "Healthy",
  "app-name": "dab_oss_2.0.0",
  "currentRole": "anonymous"
}

After:

{
  "status": "Healthy",
  "app-name": "dab_oss_2.0.0",
  "current-role": "anonymous"
}

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

GET /health

Response now contains current-role instead of currentRole.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

…for consistency

Co-authored-by: Aniruddh25 <3513779+Aniruddh25@users.noreply.github.com>
Copilot AI changed the title [WIP] [Bug] Rename currentRole to current-role for consistency fix: rename currentRole to current-role in health check response Mar 18, 2026
Copilot AI requested a review from Aniruddh25 March 18, 2026 00:08
@Aniruddh25 Aniruddh25 marked this pull request as ready for review March 18, 2026 00:28
Copilot AI review requested due to automatic review settings March 18, 2026 00:28
@Aniruddh25
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Aligns the /health endpoint JSON response keys by renaming the current role field to kebab-case for consistency with the rest of the comprehensive health report.

Changes:

  • Renamed health report JSON property from currentRole to current-role.
  • Updated the inline comment in the health response writer to reflect the new key name.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Service/HealthCheck/Model/ComprehensiveHealthCheckReport.cs Renames the serialized JSON field for CurrentRole to current-role.
src/Service/HealthCheck/ComprehensiveHealthReportResponseWriter.cs Updates comment text referencing the renamed response field.

@RubenCerna2079
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

@souvikghosh04 souvikghosh04 left a comment

Choose a reason for hiding this comment

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

LGTM

@Aniruddh25
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@souvikghosh04
Copy link
Contributor

@copilot some tests are failing. can you take a look and analyze and provide the summary here?both tests- HotReloadConfigConnectionString and HotReloadConfigDatabaseType seems to fail in assertion- "Assert.AreEqual failed. Expected:. Actual:."

…ConfigDatabaseType tests

Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot some tests are failing. can you take a look and analyze and provide the summary here?both tests- HotReloadConfigConnectionString and HotReloadConfigDatabaseType seems to fail in assertion- "A...

The failures in HotReloadConfigConnectionString and HotReloadConfigDatabaseType are not caused by the currentRolecurrent-role rename (which only affects the /health response JSON). They're a pre-existing race condition:

Root cause: The "Validated hot-reloaded configuration file" success message is logged inside RaiseChanged() before the METADATA_PROVIDER_FACTORY_ON_CONFIG_CHANGED event fires. Between those two points, a REST request can arrive and fail because the metadata providers have been cleared but not yet re-initialized. This is the same race condition that was already fixed in HotReloadConfigDataSource (in eb5408e) with a retry loop.

Fix applied in d0d1d0c: Added the same retry-with-delay pattern (up to 10 attempts, 1s apart) to HotReloadConfigConnectionString and HotReloadConfigDatabaseType for the final /rest/Book call, consistent with the existing fix in HotReloadConfigDataSource.

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