Skip to content

DT-2942: Remove Ontology checks and configurations#2877

Merged
rushtong merged 1 commit intodevelopfrom
gr-DT-2942-rm-ontology-checks
Apr 27, 2026
Merged

DT-2942: Remove Ontology checks and configurations#2877
rushtong merged 1 commit intodevelopfrom
gr-DT-2942-rm-ontology-checks

Conversation

@rushtong
Copy link
Copy Markdown
Contributor

@rushtong rushtong commented Apr 24, 2026

Addresses

https://broadworkbench.atlassian.net/browse/DT-2942

Summary

Mostly code removal with some test refactoring. Consent no longer talks to Ontology so we can remove all status checks and configurations that used it.


Have you read CONTRIBUTING.md lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@sonarqubecloud
Copy link
Copy Markdown

@rushtong rushtong marked this pull request as ready for review April 24, 2026 16:19
@rushtong rushtong requested a review from a team as a code owner April 24, 2026 16:19
@rushtong rushtong requested review from Copilot, eweitz and otchet-broad and removed request for a team April 24, 2026 16:19
Copy link
Copy Markdown
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

Removes the now-unused Ontology health check/configuration from Consent, updating status handling and tests to reflect that Consent no longer depends on the Ontology service for system health reporting.

Changes:

  • Removed Ontology health check implementation, registration, and related configuration field (ontologyURL).
  • Updated StatusResource degraded-status computation to no longer consider Ontology.
  • Refactored/removed tests that previously exercised Ontology health checking.

Reviewed changes

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

Show a summary per file
File Description
src/test/resources/consent-config.yml Removes services.ontologyURL from test config.
src/test/java/org/broadinstitute/consent/http/resources/StatusResourceTest.java Updates status tests to stop referencing Ontology health checks.
src/test/java/org/broadinstitute/consent/http/health/OntologyHealthCheckTest.java Deletes the Ontology health check test suite.
src/main/java/org/broadinstitute/consent/http/resources/StatusResource.java Removes Ontology from degraded-health evaluation.
src/main/java/org/broadinstitute/consent/http/health/OntologyHealthCheck.java Deletes the Ontology health check implementation.
src/main/java/org/broadinstitute/consent/http/configurations/ServicesConfiguration.java Removes the ontologyURL field and its accessors.
src/main/java/org/broadinstitute/consent/http/ConsentApplication.java Stops registering the Ontology health check and removes the constant/import.
Comments suppressed due to low confidence (3)

src/test/java/org/broadinstitute/consent/http/resources/StatusResourceTest.java:63

  • In testUnhealthyES, the inline comment still refers to a failing ontology check, but the test now sets the ElasticSearch check unhealthy. Update the comment to match the scenario being tested (and ideally what behavior is expected when ES is down).
    Response response = statusResource.getStatus();
    // A failing ontology check should not fail the status
    assertEquals(200, response.getStatus());

src/test/java/org/broadinstitute/consent/http/resources/StatusResourceTest.java:64

  • testUnhealthyES currently only asserts the HTTP status code, but it doesn't verify the key behavior specific to this scenario (e.g., that the response is marked degraded when ES is unhealthy). Consider asserting the degraded flag and setting the other checks (GCS/SAM/SendGrid) healthy so the test isolates ES as the cause.
  void testUnhealthyES() {
    Result elasticSearch = Result.unhealthy("ES is down");
    SortedMap<String, Result> checks = new TreeMap<>();
    checks.put(DB_ENV, Result.healthy());
    checks.put(ConsentApplication.ES_CHECK, elasticSearch);
    StatusResource statusResource = initStatusResource(checks);

    Response response = statusResource.getStatus();
    // A failing ontology check should not fail the status
    assertEquals(200, response.getStatus());
  }

src/test/java/org/broadinstitute/consent/http/resources/StatusResourceTest.java:90

  • testDegraded doesn’t include a SendGrid check entry, so formatResults will treat SendGrid as unhealthy by default, making the system degraded even if Sam were healthy. Add an explicit healthy SendGrid check (and keep the other systems healthy) so the test specifically validates that Sam being down causes degradation.
  void testDegraded() {
    SortedMap<String, Result> checks = new TreeMap<>();
    checks.put(DB_ENV, Result.healthy());
    checks.put(ConsentApplication.ES_CHECK, Result.healthy());
    checks.put(ConsentApplication.GCS_CHECK, Result.healthy());
    checks.put(ConsentApplication.SAM_CHECK, Result.unhealthy("Sam is Down"));
    StatusResource statusResource = initStatusResource(checks);

Copy link
Copy Markdown
Contributor

@otchet-broad otchet-broad left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Copy Markdown
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

👍

@rushtong rushtong merged commit c31485b into develop Apr 27, 2026
18 checks passed
@rushtong rushtong deleted the gr-DT-2942-rm-ontology-checks branch April 27, 2026 13:29
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