add spring boot integration #83
Conversation
|
@reta: do you know why |
this adds an `AutoConfiguration` for spring which automatically triggers the liquibase migration if this library is added to a project which already has spring boot on the classpath. the dependency to spring boot is marked as optional, thus it is not added as a transitive dependency to consumers which do not use spring boot. this needs to be added here until a more generic solution exists directly within spring. see spring-projects/spring-boot#37936 for more details. the test coverage for this is a bit small since we currently don't have access to the instanciated `OpenSearchClient` and would instead have to create one on our own just for the test. in the best case we'd be able to share some code with [`spring-data-opensearch-testcontainers`] (or pull it in as a dependency) to support [`@ServiceConnection`] usage. however, it seems that this library hasn't yet been published to maven, thus this isn't possible and copying over all the code might not make that much sense. for more information see these links: - [general information on spring auto-configuration](https://docs.spring.io/spring-boot/reference/using/auto-configuration.html) - [creating your own auto-configuration](https://docs.spring.io/spring-boot/reference/features/developing-auto-configuration.html) [`spring-data-opensearch-testcontainers`]: https://github.com/opensearch-project/spring-data-opensearch/tree/main/spring-data-opensearch-testcontainers [`@ServiceConnection`]: https://docs.spring.io/spring-boot/reference/testing/testcontainers.html#testing.testcontainers.service-connections
no need to hand-roll this if there's a built-in alternative.
rursprung
force-pushed
the
add-spring-integration
branch
from
7b90a70
to
7ec5ceb
Compare
|
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-simple</artifactId> | ||
| <version>2.0.16</version> | ||
| <groupId>org.springframework.boot</groupId> |
There was a problem hiding this comment.
It seems like liquibase does not follow the convention, but it would be great to have spring-boot-liquibase-opensearch-starter / liquibase-opensearch-starter as a separate module
| @@ -144,7 +145,7 @@ private void connect() { | |||
| try { | |||
| sslcontext = SSLContextBuilder | |||
| .create() | |||
| .loadTrustMaterial(null, (chains, authType) -> true) | |||
| .loadTrustMaterial(null, new TrustAllStrategy()) | |||
There was a problem hiding this comment.
I think this is fine for tests but not production, the `TrustAllStrategy is unsafe
There was a problem hiding this comment.
this is unrelated to this PR (sorry, i just snuck it in here, but at least as an atomic commit) - this commit doesn't change the behaviour at all, it's just making it more obvious what's happening here. there's #29 to improve this situation (but no clear idea on how to solve it yet).
| @AutoConfiguration | ||
| @EnableConfigurationProperties(SpringLiquibaseOpenSearchProperties.class) | ||
| @ConditionalOnProperty("opensearch.uris") | ||
| public class LiquibaseOpenSearchAutoConfiguration { |
There was a problem hiding this comment.
I think we need to account for OpenSearch related beans (clients, etc) to be present (or not) in the context, I am happy to help you there (if it makes sense)
There was a problem hiding this comment.
liquibase-opensearch already pulls in the opensearch client as a dependency, so the beans will always be here; thus i don't think that we need the conditional?
any help is welcome, of course! 👍 the faster we get this working the better for everyone
@rursprung hm ... it was released https://central.sonatype.com/artifact/org.opensearch.client/spring-data-opensearch-testcontainers, no?
I think we could, why not? (if it is helpful, and also it would be test only dependency) |
interesting, i can't seem to find it on maven central: https://mvnrepository.com/search?q=spring-data-opensearch-testcontainers |
|
as discussed this will most likely move to a dedicated repo, see #86 for further details |
this adds an
AutoConfigurationfor spring which automatically triggersthe liquibase migration if this library is added to a project which
already has spring boot on the classpath.
the dependency to spring boot is marked as optional, thus it is not
added as a transitive dependency to consumers which do not use spring
boot.
this needs to be added here until a more generic solution exists
directly within spring. see spring-projects/spring-boot#37936 for more
details.
the test coverage for this is a bit small since we currently don't have
access to the instanciated
OpenSearchClientand would instead have tocreate one on our own just for the test.
in the best case we'd be able to share some code with
spring-data-opensearch-testcontainers(or pull it in as adependency) to support
@ServiceConnectionusage. however, it seemsthat this library hasn't yet been published to maven, thus this isn't
possible and copying over all the code might not make that much sense.
for more information see these links: