-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Remove invalid module from cron job #10842
Conversation
@geoand instead of removing the entry, please add the correct one. |
@@ -54,7 +54,7 @@ jobs: | |||
run: mvn -B install -DskipTests -DskipITs -Dformat.skip | |||
|
|||
- name: Run integration tests in native | |||
run: mvn -B --settings .github/mvn-settings.xml verify -f integration-tests/pom.xml --fail-at-end -Dno-format -Ddocker -Dnative -Dquarkus.native.container-build=true -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-native-image:20.1.0-java${{ matrix.java }} -Dtest-postgresql -Dtest-elasticsearch -Dtest-mysql -Dtest-db2 -Dtest-amazon-services -Dtest-vault -Dtest-neo4j -Dtest-keycloak -Dtest-kafka -Dtest-mssql -Dtest-mariadb -Dmariadb.url="jdbc:mariadb://localhost:3308/hibernate_orm_test" -pl '!io.quarkus:quarkus-integration-test-google-cloud-functions-http,!io.quarkus:quarkus-integration-test-google-cloud-functions,!integration-tests/funqy-google-cloud-function' | |||
run: mvn -B --settings .github/mvn-settings.xml verify -f integration-tests/pom.xml --fail-at-end -Dno-format -Ddocker -Dnative -Dquarkus.native.container-build=true -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-native-image:20.1.0-java${{ matrix.java }} -Dtest-postgresql -Dtest-elasticsearch -Dtest-mysql -Dtest-db2 -Dtest-amazon-services -Dtest-vault -Dtest-neo4j -Dtest-keycloak -Dtest-kafka -Dtest-mssql -Dtest-mariadb -Dmariadb.url="jdbc:mariadb://localhost:3308/hibernate_orm_test" -pl '!io.quarkus:quarkus-integration-test-google-cloud-functions-http,!io.quarkus:quarkus-integration-test-google-cloud-functions' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum. I didn't see that but I really don't like the fact that we exclude these modules. We are supposed to support native everywhere.
@loicmathieu what's the reason of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google Cloud Functions only runs inside the Google Cloud Function Java runtime which is a Jetty server.
That's why it didn't runs in native.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, there is no point of running a Function in native, this will no longuer be a function as we will need to include a runtime (wich one ?), create a bridge from the function API to this runtime, find a way to listen to the function event (PubSub, Storage, ...).
So we will end up re-implementing the function runtime provided by Google ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so couldn't we configure it in the pom of these modules instead (if configuration is needed but I suppose it will just build fine and won't build a native image)? I mean it's really no biggie to build these projects, it's really not something that will take time compared to building the other native images. And preferable to having to maintain this list IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Azure functions didn't run on native neither ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google Cloud Functions only works in Java 11. Native build is Java 8 that's why we exlude the ITs from the build.
Same is done on the standard CI, the Google Cloud Functions ITs are also excluded there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Merging this for now to get the cron job passing. We can figure out a better approach later if needed |
Fixes: #6717
The entry was responsible for: