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

Feat/upgrade deps and macos build #215

Conversation

clemlesne
Copy link

@clemlesne clemlesne commented Nov 25, 2022

Hello,

Additions of my proposal:

  • Upgrade: Maven, dependencies & container build
  • Quality: Code quality
  • Feat: Add healthchecks for every services
  • Fix: Local run on macOS
  • Perf: Enhance performances with config local caching
  • CI: Add validation step, clean code, use local Maven install

Regards,

@arey
Copy link
Member

arey commented Nov 25, 2022

Thanks a lot Clémence for your very interesting contribution. I will do a review
For the next time (I hope there will), I'll prefer smaller PR :)


## Prerequisites

Be warning to use the right Java version (latest LTS, v17):
Copy link
Member

Choose a reason for hiding this comment

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

Java 17 is the baseline of Spring Boot 3: https://spring.io/blog/2022/11/24/spring-boot-3-0-goes-ga
We have to upgrade. But for now I think we could support Java 11 and 17

Copy link
Author

Choose a reason for hiding this comment

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

I didn't tested the build with both versions. I think it'll add unnecessary complexity to the code. It is really required to support a version which is not the latest LTS?

Copy link
Member

Choose a reason for hiding this comment

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

Some Java developers are using this sample in their compagny where Java 17 may be not still available in live environment. It's quite commun in big non-IT organization. Thus keeping Java 11 compatibility could be comforting.

README.md Outdated
so don't be scared of initial Spring Cloud Gateway timeouts. You can track services availability using Eureka dashboard
available by default at http://localhost:8761.
In order to start entire infrastructure using Docker, you have to build images by executing `./mvnw clean spring-boot:build-image`
from a project root. Once images are ready, you can start them with a single command `docker-compose up`. Containers startup order is coordinated with [`dockerize` script](https://github.com/jwilder/dockerize).
Copy link
Member

Choose a reason for hiding this comment

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

Does we still use dockerize?

Copy link
Author

Choose a reason for hiding this comment

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

I'll update the README accordingly.

Copy link
Author

Choose a reason for hiding this comment

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

Fix proposal in 4a6c316

@spring-petclinic spring-petclinic deleted a comment from clemlesne Nov 28, 2022

## Third-parties

sql-server:
Copy link
Member

Choose a reason for hiding this comment

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

Previous version of the Spring Petclinic started by Docker Compose was using a HSQL database and not a MySQL database.
From your git branch, I've tested the docker-compose file with the docker-compose up command.
The whole application didn't start. And I've got this error:

vets-service  | 2022-11-29 07:13:50.781 ERROR [vets-service,,] 1 --- [           main] com.zaxxer.hikari.pool.HikariPool        : HikariPool-1 - Exception during pool initialization.
vets-service  |
vets-service  | com.mysql.cj.jdbc.exceptions.CommunicationsException: Communications link failure
vets-service  |
vets-service  | The last packet sent successfully to the server was 0 milliseconds ago. The driver has not received any packets from the server.
vets-service  |         at com.mysql.cj.jdbc.exceptions.SQLError.createCommunicationsException(SQLError.java:174)

Does that mean anything to you?

@arey arey mentioned this pull request Dec 23, 2022
@arey
Copy link
Member

arey commented Dec 21, 2023

Clémence MacOS build on ARM architecture has been completed in the #242
Feel free to submit other improvements in smaller PR please

@arey arey closed this Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants