feat: Database configuration integration with Spring Boot #1481
Conversation
|
@jmformenti Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@jmformenti Thank you for signing the Contributor License Agreement! |
There was a problem hiding this comment.
You should also update the:/petclinic_db_setup_postgres.txt
and/petclinic_db_setup_mysql.txt
accordingly :)
Removed that documentation because with these changes no manual steps are needed anymore. |
pom.xml
Outdated
| @@ -68,6 +68,10 @@ | |||
| <artifactId>spring-boot-starter-test</artifactId> | |||
| <scope>test</scope> | |||
| </dependency> | |||
| <dependency> | |||
There was a problem hiding this comment.
I'm not super happy about having this not in test scope. The CLI examples would all work with mvn spring-boot:test-run would they not?
There was a problem hiding this comment.
Good point, It doesn't work directly because of this:
[ERROR] Failed to execute goal org.springframework.boot:spring-boot-maven-plugin:3.2.1:test-run (default-cli) on project spring-petclinic: Execution default-cli of goal org.springframework.boot:spring-boot-maven-plugin:3.2.1:test-run failed: Unable to find a single main class from the following candidates [org.springframework.samples.petclinic.PostgresIntegrationTests, org.springframework.samples.petclinic.PetClinicIntegrationTests, org.springframework.samples.petclinic.MysqlTestApplication]
but I've modified the PR to keep the docker compose dependency in test scope.
jmformenti
force-pushed
the
feature/database-configuration-integration-spring-boot
branch
from
21963ae
to
5aedb47
Compare
|
Looking at it in more detail I'm not really sure I know what the intention was here. I don't want docker-compose to be the only option to run a database, and that seems to be the main idea behind the change? Was there something else I missed? |
jmformenti
force-pushed
the
feature/database-configuration-integration-spring-boot
branch
from
da50c9e
to
a9bcd80
Compare
Yes, the idea was essentially to simplify database configuration by leveraging the integration with docker compose, but I understand your point of view. |
|
Since we already support docker-compose for both mysql and postgres, this might end up being an empty pull request, once we whittle it down to what is actually needed. So far that looks like a few lines of documentation and some Maven profiles (but Gradle support is still missing, which we probably need if we are providing Maven examples). The details of what is in those profiles is still being debated, but I'm not convinced they are all that helpful. Maybe there is a problem with the documentation making it hard to understand the steps needed to run using docker compose? It could be that all we need is more clarity in the README? |
|
Personally I like the idea to run the application and the database with a single command: with any other |
|
I usually run from the IDE, so I'm not that bothered, but if that is the objective then it seems reasonable to me. Whatever we decide to do, it shouldn't take much code, IMO. We also haven't tried to cover the Gradle users yet, so there's a bit more work to do still. Lastly, I'm not sure if we can do exactly the same for MySQL as Postgres since one of them (currently MySQL) is being used to showcase test containers and the other is showing docker compose. I would be happy if If you're not able to complete the PR that's OK. I'll leave it open for a bit to see if anyone else feels like doing it. |
Simplified database configuration using the new Spring Boot Docker Compose feature.
You can use MySQL or PostgreSQL by simply defining the Spring Boot profile:
or