Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Spring Boot Docker Compose support #61

Merged
merged 5 commits into from
Jun 18, 2024

Conversation

robertmcnees
Copy link
Contributor

@robertmcnees robertmcnees commented May 31, 2024

This PR restructures a lot of the guide to use Spring Boot Docker Compose support. They key teachings of the guide remain the same, however a lot of the upfront tasks, such as creating the MySQL database and the application.properties file, are no longer at the beginning of the guide. These tasks are no longer required, which should allow the user to get started with the code faster than before.

There are a few specific items in this PR that I would like a further review on:

  • The current CI build is broken due to a test failure in the initial folder. This test does not exist at all in the complete folder. I added Testcontainers to the project so that the context loads and these tests pass. The test now exists in the complete and initial folders.
  • I've added text around Spring Boot Docker Compose support that I largely copied from the Spring Boot reference documentation. As this will likely become common text throughout several guides, an in depth review is appreciated. The common file text is located here.
  • In the new Building the Application section, I am using the Docker feature for container networks so that all of my containers can communicate with one another. Is this the best practice? I also had to add a third container to test the application, as specified in the new Test the Application in Docker section.
  • I did not render the instructions on native image for this guide, as I received an error from Hibernate at runtime. If native does not work with no custom hints, I think it is best not to add that complexity. Still, I find it interesting native did not work out of the box so it could be something I am doing wrong. (Update: native compilation now works with this guide, when using the 0.10.2 version of the Gradle plugin)

Hoping that @mp911de and @mhalbritter can take a look.

Addresses issue #60

@robertmcnees robertmcnees self-assigned this May 31, 2024
README.adoc Outdated Show resolved Hide resolved
README.adoc Show resolved Hide resolved
@mhalbritter
Copy link

Hey Bob,

thanks for your work! I've added some comments, but they are only minor things.

@mhalbritter
Copy link

Regarding the common file:

Search for a compose.yml and other common compose filenames in your working directory

compose.yml should be in backticks, compose.yml.

Call Docker compose up with the discovered compose.yml

I'd add backticks:

Call docker compose up with the discovered compose.yml

Call Docker compose stop when the application is shutdown

I'd add backticks:

Call docker compose stop when the application is shutdown

The current guide instructs the user to run a command
docker-compose but the more recent version of the command
is docker compose.
README.adoc Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Show resolved Hide resolved

== Test the Application in Docker

If you ran the application using a Docker instruction above, a simple curl command from a terminal or command line will no longer work.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change "a Docker instruction above" to "the Docker instruction shown earlier"

README.adoc Outdated Show resolved Hide resolved
@robertmcnees robertmcnees merged commit 655df69 into spring-guides:main Jun 18, 2024
2 checks passed
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.

3 participants