quarkusio / quarkus-quickstarts

Quarkus quickstart code
https://quarkus.io
Apache License 2.0
1.97k stars 1.47k forks source link

getting-started: Clean up s2i/environment #1359

Closed jmtd closed 7 months ago

jmtd commented 11 months ago

Here we remove some environment variable definitions from .s2i/environment which used to be necessary for the Red Hat OpenJDK container images to work correctly with the default Quarkus "fast-jar" package type. The drawback of defining these variables is that the quickstart now only works for the fast-jar layout, and would break if the user specifies another via QUARKUS_PACKAGE_TYPE, unless they realise .s2i/environment (a hidden file) exists, and either alter it or override the other variables within.

The RH OpenJDK container image works with fast-jar OOTB since :1.17 (2023-08-28).

Whilst I was in there, I removed AB_JOLOKIA_OFF, which is not needed for the JDK17 or JDK21 images; and changed JAVA_OPTIONS to JAVA_OPTS_APPEND, which I think has the desired semantics. That last change in particular I would like some feedback on:

Check list:

Your pull request:

cescoffier commented 11 months ago

Can you open the PR against the development branch instead of main?

jmtd commented 11 months ago

Can you open the PR against the development branch instead of main?

Sure, I'll rebase now (done in ac1eadc866bc3dbba41b90eaba5285dd0e55058c); but as I said, this quickstart appears to be failing in development right now, before my patches, so it's harder to verify:

I1211 12:32:03.697855  112432 sti.go:713] [ERROR]     'dependencies.dependency.version' for io.quarkus:quarkus-resteasy-reactive:jar is missing. @ line 34, column 21
I1211 12:32:03.697896  112432 sti.go:713] [ERROR]     'dependencies.dependency.version' for io.quarkus:quarkus-junit5:jar is missing. @ line 38, column 21
I1211 12:32:03.697954  112432 sti.go:713] [ERROR]     'dependencies.dependency.version' for io.rest-assured:rest-assured:jar is missing. @ line 43, column 21

or perhaps that's expected on dev branch? Since 999-SNAPSHOT is a placeholder? Edit: ah, once I've built quarkus master branch (not development) I have the necessary artefacts locally to run mvn verify etc.

jmtd commented 7 months ago

Can anyone take a look at this please?