Feature/elastic on boot #856
Conversation
|
This Pull Request has failed the formatting check Please run You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Relates to hapifhir/hapi-fhir#7242 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Formatting check succeeded! |
|
Formatting check succeeded! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Formatting check succeeded! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Formatting check succeeded! |
|
Formatting check succeeded! |
|
Formatting check succeeded! |
There was a problem hiding this comment.
Pull Request Overview
This PR rewires Elasticsearch configuration to follow Spring Boot's standard approach for Elasticsearch integration. The changes replace custom Elasticsearch configuration with Spring Boot's auto-configuration and update related components.
Key changes:
- Replace custom Elasticsearch configuration with Spring Boot's standard approach
- Migrate from custom condition classes to
@ConditionalOnPropertyannotations - Update Elasticsearch service implementation to use Spring Boot's ElasticsearchClient
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/application-test.yaml | Remove extensive configuration, simplifying test setup |
| Multiple test files | Add Hibernate search backend configuration for local testing |
| src/main/resources/application.yaml | Restructure Hibernate and Elasticsearch configuration to use Spring Boot patterns |
| EnvironmentHelper.java | Remove custom Hibernate properties configuration logic |
| Multiple config classes | Replace custom condition classes with @ConditionalOnProperty annotations |
| ElasticsearchBootSvcImpl.java | New Elasticsearch service implementation using Spring Boot's client |
| Application.java | Enable Spring Boot's Elasticsearch auto-configuration |
| pom.xml | Move Elasticsearch dependencies from test to main scope |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| @ExtendWith(SpringExtension.class) | ||
| @Testcontainers | ||
| @Disabled |
There was a problem hiding this comment.
Is this intended to be left disabled? :-)
There was a problem hiding this comment.
Yes - its marginally better that having this:
The workflow succeeds giving a green check-mark next to the commit id. Perhaps writing a comment each time is redundant? E.g.: hapifhir#856
* ci: remove success message from formatting check The workflow succeeds giving a green check-mark next to the commit id. Perhaps writing a comment each time is redundant? E.g.: #856 * Update spotless check workflow to handle failures Modify condition for PR comment action to run only on failure.
# Conflicts: # src/main/resources/application.yaml
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public void mcpTests() throws JsonProcessingException { | ||
|
|
||
| var fhirContext = FhirContext.forR4(); | ||
|
|
There was a problem hiding this comment.
The endpoint change from '/mcp/message' to '/mcp/messages' should be documented in the test or PR description to clarify whether this is an intentional API change or a correction to match the new MCP configuration.
| // The endpoint '/mcp/messages' is used here instead of '/mcp/message' to match the current MCP configuration. | |
| // This is an intentional change reflecting the updated API endpoint. |
Copilot uses AI. Check for mistakes.
src/main/java/ca/uhn/fhir/jpa/starter/elastic/ElasticsearchBootSvcImpl.java
Show resolved
Hide resolved
Rewiring the use of elastic search to follow usual Spring Boot way of doing it