-
Notifications
You must be signed in to change notification settings - Fork 352
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
Use Quarkus fast-jar package format apache/camel-k-runtime#360 #1931
Conversation
Very nice! It may impact #1816. |
Will this work with local dependency resolution? It would be good to at least try it out with the set of local commands which use the current way of resolving dependencies. For example running |
I did test that and it looked to be working ok. Also ran the 'local' integration test suite and it passed. |
Actually now I think about it a bit more, maybe it isn't.... The paths listed are to the temp maven build directory, which I guess is not correct (since it is 'temporary')? I suppose the paths are meant to resolve to the artifacts in the local m2 repository? |
If that's the case then we now have even more need of a means of saving the dependencies after the command is completed and the temporary folder is removed. |
So I think that today we have the same issue with the jar containing the quarkus-generated main function. If I understand correctly this will now apply to the rest of the dependencies too. |
Here's some example output from
|
I see, I mean that would be ok for the narrow purposes of the inspect command but unless they can be saved in another folder then those dependencies will not be there after the command completes. The question is, when trying to containerize the application. If you run:
Does the image run successfully? |
Yes, that part works. The integration tests verify that and I did some of my own testing on it. |
Awesome! |
8c569ff
to
5360ded
Compare
Seems a Spectrum tweak is also required, so opened container-tools/spectrum#4. |
94627fd
to
3c25121
Compare
1259c4a
to
512ef2e
Compare
I don't understand why there are random failures in the YAKS knative tests. They don't occur for me locally. I've put this PR back into review and it's good to merge IMO. |
Thanks for the update. Yes, there are some flaky YAKS tests, that are not caused by your changes. |
@jamesnetherton could you please resolve the conflicts? |
Conflicts resolved. |
@astefanutti are we good to merge this one? The usual flaky tests failed. |
@jamesnetherton let's retry one last time, and have it merged. |
@jamesnetherton one last point, as the metrics endpoint exposed by Quarkus has its path changed, I think the /cc @llowinge who wrote the monitoring e2e tests. |
Added the additional config to the |
Perfect, LGTM 👍🏼. |
Woah! A successful build 😅 |
I'll merge as I'd like to not have to rebase this one again... |
Depends on an as-yet uncommitted change to camel-k-runtime (hence draft PR), but this PR provides the means to support the Quarkus fast-jar format.
At the moment fast-jar expects a specific directory structure, so I've had to replicate that in the container under
/deployment/dependencies
.Also I assumed it's safe to not use the
generate-dependency-list
mojo since thequarkus-maven-plugin
already does its own dependency resolution and downloading of artifacts.Release Note