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

Adds micronaut #160

Merged
merged 4 commits into from
Nov 12, 2018
Merged

Conversation

lordofthejars
Copy link
Collaborator

No description provided.

@kameshsampath
Copy link
Contributor

@lordofthejars - i see that you have combined GraalVM and micronaut together. if you dont mind can you please split them in to two different PRs, that will be easy for managing and reversing.

@lordofthejars
Copy link
Collaborator Author

@kameshsampath Well currently adding Graal is just adding a Docker image that recompiles your jar file to a native file, so there are no big changes at all.

Also micronaut scaffolds the application at once, I mean you don't do anything manually, you just say I want to create a micronaut app with graal and it creates everything (the docker image + the app), so it is really simple. In any case, since now I am going to update the graal version you will get separated, but as I said the only difference between using graal or not is just a docker image.

@kameshsampath
Copy link
Contributor

@lordofthejars thanks for the explanation, thinking why cant we add something like Dockerfile.graal to all the microservices ? In that way I feel we can say to the interested people that graal is for every app irrespective of the framework we use. IMHO in this case since we use only for micronaut I wonder developers will be confused that graal is only for micronaut.

Do I make sense ?

@lordofthejars
Copy link
Collaborator Author

lordofthejars commented Nov 11, 2018 via email

@lordofthejars
Copy link
Collaborator Author

lordofthejars commented Nov 11, 2018 via email

@kameshsampath
Copy link
Contributor

kameshsampath commented Nov 11, 2018

The problem is that graal does not work with everything, just with projects
that do not use reflection. Currently micronaut and probably others but not
a lot.

ok. I will merge, can you please update the README with a NOTE/WARNING saying this ?

@lordofthejars
Copy link
Collaborator Author

lordofthejars commented Nov 11, 2018 via email

@lordofthejars
Copy link
Collaborator Author

@kameshsampath Done, now there is a warning in the docs.

Copy link
Contributor

@kameshsampath kameshsampath left a comment

Choose a reason for hiding this comment

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

Also since there are lot of common dependencies across the three microservices will not be easy to add parent pom ?

customer/java/micronaut/Dockerfile Show resolved Hide resolved
customer/java/micronaut/Dockerfile Show resolved Hide resolved
customer/java/micronaut/README.adoc Show resolved Hide resolved
preference/java/micronaut/Dockerfile Show resolved Hide resolved
preference/java/micronaut/Dockerfile Show resolved Hide resolved
recommendation/java/micronaut/Dockerfile Show resolved Hide resolved
@lordofthejars
Copy link
Collaborator Author

@kameshsampath about parent pom, well it is the same that happens in Spring Boot, Vertx, microprofile which all services share the same dependencies. Since all three services are in different places, I think that adding a parent pom will add some complexity because when you do the maven package you will need to have this parent pom already published (parent pom will be in a common place between all services) so it means adding an extra step on the build. Also, the examples, in general, are really simple so apart from adding an extra step it will be a bit more complex to understand, now as it happens to any other service we have in the tutorial is self packaged.

@kameshsampath kameshsampath merged commit b56cc91 into redhat-scholars:master Nov 12, 2018
@lordofthejars
Copy link
Collaborator Author

@kameshsampath yes multistage does not work in Minishift, for this reason, I have added a Dockerfile-openshift. Thanks for the review.

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.

2 participants