Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades merged PR #4827: URL: https://github.com/apache/gravitino/pull/4827 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
yuqi1129 commented on PR #4827: URL: https://github.com/apache/gravitino/pull/4827#issuecomment-2348107834 > Any more comments here @yuqi1129 ? I have no further comments on the matter. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
jerryshao commented on PR #4827: URL: https://github.com/apache/gravitino/pull/4827#issuecomment-2348015931 Any more comments here @yuqi1129 ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756475125
##
build.gradle.kts:
##
@@ -204,6 +204,13 @@ allprojects {
throw GradleException("Gravitino integration tests only support
[-PtestMode=embedded] or [-PtestMode=deploy] mode!")
}
+ param.useJUnitPlatform()
Review Comment:
No. line 214 is an overwrite, removing this will make `skipUT` not work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756471584
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
The current way will only generate one CI job, if it is written in matrix,
it will generate three CI jobs, which will take up the concurrency of the CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
yuqi1129 commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756438417
##
build.gradle.kts:
##
@@ -204,6 +204,13 @@ allprojects {
throw GradleException("Gravitino integration tests only support
[-PtestMode=embedded] or [-PtestMode=deploy] mode!")
}
+ param.useJUnitPlatform()
Review Comment:
This line seems to be repeated with L214.
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
You can exclude someone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
FANNG1 commented on PR #4827: URL: https://github.com/apache/gravitino/pull/4827#issuecomment-2345536290 LGTM -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756293374 ## .github/workflows/flink-integration-test.yml: ## @@ -28,77 +27,45 @@ jobs: - catalogs/** - clients/client-java/** - clients/client-java-runtime/** - - clients/filesystem-hadoop3/** - - clients/filesystem-hadoop3-runtime/** - common/** - conf/** - core/** - dev/** + - flink-connector/** - gradle/** + - iceberg/** Review Comment: removed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
FANNG1 commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756283538 ## .github/workflows/flink-integration-test.yml: ## @@ -28,77 +27,45 @@ jobs: - catalogs/** - clients/client-java/** - clients/client-java-runtime/** - - clients/filesystem-hadoop3/** - - clients/filesystem-hadoop3-runtime/** - common/** - conf/** - core/** - dev/** + - flink-connector/** - gradle/** + - iceberg/** Review Comment: Iceberg support for Flink is not on the roadmap, I prefer to remove it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756280900 ## .github/workflows/flink-integration-test.yml: ## @@ -28,77 +27,45 @@ jobs: - catalogs/** - clients/client-java/** - clients/client-java-runtime/** - - clients/filesystem-hadoop3/** - - clients/filesystem-hadoop3-runtime/** - common/** - conf/** - core/** - dev/** + - flink-connector/** - gradle/** + - iceberg/** Review Comment: Maybe this can be solved through comments, let me try -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756279558 ## .github/workflows/flink-integration-test.yml: ## @@ -28,77 +27,45 @@ jobs: - catalogs/** - clients/client-java/** - clients/client-java-runtime/** - - clients/filesystem-hadoop3/** - - clients/filesystem-hadoop3-runtime/** - common/** - conf/** - core/** - dev/** + - flink-connector/** - gradle/** + - iceberg/** Review Comment: Would you recommend adding this line of code after Flink supports Iceberg? I'm worried that I'll forget about it then, as it seems easy to miss -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756273223
##
catalogs/catalog-lakehouse-iceberg/build.gradle.kts:
##
@@ -138,16 +138,10 @@ tasks {
}
tasks.test {
- val skipUTs = project.hasProperty("skipTests")
- if (skipUTs) {
-// Only run integration tests
-include("**/integration/**")
- }
-
val skipITs = project.hasProperty("skipITs")
if (skipITs) {
// Exclude integration tests
-exclude("**/integration/**")
Review Comment:
make it more accurate. should be `"**/integration/test/**"`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756271385
##
catalogs/catalog-lakehouse-iceberg/build.gradle.kts:
##
@@ -138,16 +138,10 @@ tasks {
}
tasks.test {
- val skipUTs = project.hasProperty("skipTests")
Review Comment:
It didn't work in this way (because miss `useJUnitPlatform()`) and I fixed
this in root kts:
https://github.com/apache/gravitino/pull/4827/files#diff-c0dfa6bc7a8685217f70a860145fbdf416d449eaff052fa28352c5cec1a98c06R207-R213
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
FANNG1 commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756255295
##
catalogs/catalog-lakehouse-iceberg/build.gradle.kts:
##
@@ -138,16 +138,10 @@ tasks {
}
tasks.test {
- val skipUTs = project.hasProperty("skipTests")
Review Comment:
why remove this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
FANNG1 commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1756248592 ## .github/workflows/flink-integration-test.yml: ## @@ -28,77 +27,45 @@ jobs: - catalogs/** - clients/client-java/** - clients/client-java-runtime/** - - clients/filesystem-hadoop3/** - - clients/filesystem-hadoop3-runtime/** - common/** - conf/** - core/** - dev/** + - flink-connector/** - gradle/** + - iceberg/** Review Comment: Flink doesn't rely on `iceberg` currently. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1755991259
##
.github/workflows/backend-integration-test-action.yml:
##
@@ -0,0 +1,77 @@
+name: Backend Integration Test Action
+
+# run backend integration test
+on:
+ workflow_call:
+inputs:
+ architecture:
+required: true
+description: 'Architecture of the platform'
+type: string
+ java-version:
+required: true
+description: 'Java version'
+type: string
+ backend:
+required: true
+description: 'Backend storage for Gravitino'
+type: string
+ test-mode:
+required: true
+description: 'run on embedded or deploy mode'
+type: string
+
+jobs:
+ start-runner:
+name: JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{
inputs.backend }}
+runs-on: ubuntu-latest
+timeout-minutes: 90
+env:
+ PLATFORM: ${{ inputs.architecture }}
+steps:
+ - uses: actions/checkout@v3
+
+ - uses: actions/setup-java@v4
+with:
+ java-version: ${{ inputs.java-version }}
+ distribution: 'temurin'
+ cache: 'gradle'
+
+ - name: Set up QEMU
+uses: docker/setup-qemu-action@v2
+
+ - name: Check required command
+run: |
+ dev/ci/check_commands.sh
+
+ - name: Package Gravitino
+if: ${{ inputs.test-mode == 'deploy' }}
+run: |
+ ./gradlew compileDistribution -x test -PjdkVersion=${{
inputs.java-version }}
+
+ - name: Free up disk space
+run: |
+ dev/ci/util_free_space.sh
+
+ - name: Backend Integration Test (JDK${{ inputs.java-version }}-${{
inputs.test-mode }}-${{ inputs.backend }})
+id: integrationTest
+run: >
+ ./gradlew test -PskipTests -PtestMode=${{ inputs.test-mode }}
-PjdkVersion=${{ inputs.java-version }} -PjdbcBackend=${{ inputs.backend }}
-PskipDockerTests=false
+ -x :web:test -x :clients:client-python:test -x
:flink-connector:flink:test -x :spark-connector:test -x
:spark-connector:spark-common:test
Review Comment:
yeah, fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
jerryshao commented on PR #4827: URL: https://github.com/apache/gravitino/pull/4827#issuecomment-2344346885 @diqiu50 @FANNG1 @yuqi1129 @xunliu can you please help to review your parts again? Generally, LGTM, waiting for your check again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
jerryshao commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1755243308
##
.github/workflows/backend-integration-test-action.yml:
##
@@ -0,0 +1,77 @@
+name: Backend Integration Test Action
+
+# run backend integration test
+on:
+ workflow_call:
+inputs:
+ architecture:
+required: true
+description: 'Architecture of the platform'
+type: string
+ java-version:
+required: true
+description: 'Java version'
+type: string
+ backend:
+required: true
+description: 'Backend storage for Gravitino'
+type: string
+ test-mode:
+required: true
+description: 'run on embedded or deploy mode'
+type: string
+
+jobs:
+ start-runner:
+name: JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{
inputs.backend }}
+runs-on: ubuntu-latest
+timeout-minutes: 90
+env:
+ PLATFORM: ${{ inputs.architecture }}
+steps:
+ - uses: actions/checkout@v3
+
+ - uses: actions/setup-java@v4
+with:
+ java-version: ${{ inputs.java-version }}
+ distribution: 'temurin'
+ cache: 'gradle'
+
+ - name: Set up QEMU
+uses: docker/setup-qemu-action@v2
+
+ - name: Check required command
+run: |
+ dev/ci/check_commands.sh
+
+ - name: Package Gravitino
+if: ${{ inputs.test-mode == 'deploy' }}
+run: |
+ ./gradlew compileDistribution -x test -PjdkVersion=${{
inputs.java-version }}
+
+ - name: Free up disk space
+run: |
+ dev/ci/util_free_space.sh
+
+ - name: Backend Integration Test (JDK${{ inputs.java-version }}-${{
inputs.test-mode }}-${{ inputs.backend }})
+id: integrationTest
+run: >
+ ./gradlew test -PskipTests -PtestMode=${{ inputs.test-mode }}
-PjdkVersion=${{ inputs.java-version }} -PjdbcBackend=${{ inputs.backend }}
-PskipDockerTests=false
+ -x :web:test -x :clients:client-python:test -x
:flink-connector:flink:test -x :spark-connector:test -x
:spark-connector:spark-common:test
Review Comment:
also `:spark-connector:test` seems useless.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
jerryshao commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1755239324
##
.github/workflows/backend-integration-test-action.yml:
##
@@ -0,0 +1,77 @@
+name: Backend Integration Test Action
+
+# run backend integration test
+on:
+ workflow_call:
+inputs:
+ architecture:
+required: true
+description: 'Architecture of the platform'
+type: string
+ java-version:
+required: true
+description: 'Java version'
+type: string
+ backend:
+required: true
+description: 'Backend storage for Gravitino'
+type: string
+ test-mode:
+required: true
+description: 'run on embedded or deploy mode'
+type: string
+
+jobs:
+ start-runner:
+name: JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{
inputs.backend }}
+runs-on: ubuntu-latest
+timeout-minutes: 90
+env:
+ PLATFORM: ${{ inputs.architecture }}
+steps:
+ - uses: actions/checkout@v3
+
+ - uses: actions/setup-java@v4
+with:
+ java-version: ${{ inputs.java-version }}
+ distribution: 'temurin'
+ cache: 'gradle'
+
+ - name: Set up QEMU
+uses: docker/setup-qemu-action@v2
+
+ - name: Check required command
+run: |
+ dev/ci/check_commands.sh
+
+ - name: Package Gravitino
+if: ${{ inputs.test-mode == 'deploy' }}
+run: |
+ ./gradlew compileDistribution -x test -PjdkVersion=${{
inputs.java-version }}
+
+ - name: Free up disk space
+run: |
+ dev/ci/util_free_space.sh
+
+ - name: Backend Integration Test (JDK${{ inputs.java-version }}-${{
inputs.test-mode }}-${{ inputs.backend }})
+id: integrationTest
+run: >
+ ./gradlew test -PskipTests -PtestMode=${{ inputs.test-mode }}
-PjdkVersion=${{ inputs.java-version }} -PjdbcBackend=${{ inputs.backend }}
-PskipDockerTests=false
+ -x :web:test -x :clients:client-python:test -x
:flink-connector:flink:test -x :spark-connector:test -x
:spark-connector:spark-common:test
Review Comment:
`:web:test` should be changed to `:web:web:test`, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
yuqi1129 commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753186586 ## .github/workflows/access-control-integration-test.yml: ## @@ -22,21 +22,32 @@ jobs: with: filters: | source_changes: + - .github/** - api/** - authorizations/** + - bin/** Review Comment: OKay, it's not a big problem any how. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753149517 ## .github/workflows/spark-integration-test.yml: ## @@ -108,3 +107,64 @@ jobs: spark-connector/v3.5/spark/build/spark-3.5-integration-test.log distribution/package/logs/*.out distribution/package/logs/*.log + + SparkIT-on-pr: +needs: changes +if: (github.event_name == 'pull_request' && needs.changes.outputs.source_changes == 'true') Review Comment: @FANNG1 @yuqi1129 updated, plz help review again, thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753029446 ## .github/workflows/spark-integration-test.yml: ## @@ -108,3 +107,64 @@ jobs: spark-connector/v3.5/spark/build/spark-3.5-integration-test.log distribution/package/logs/*.out distribution/package/logs/*.log + + SparkIT-on-pr: +needs: changes +if: (github.event_name == 'pull_request' && needs.changes.outputs.source_changes == 'true') Review Comment: Since two people have suggested doing this, I'll try to extract the public part -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753025963
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
That will produce extra CI jobs, so I put it in the step
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753022176 ## .github/workflows/access-control-integration-test.yml: ## @@ -22,21 +22,32 @@ jobs: with: filters: | source_changes: + - .github/** - api/** - authorizations/** + - bin/** Review Comment: There are some environment and JVM parameters in `bin/common.sh` and `bin/gravitino.sh` that might affect the deployment test? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
yuqi1129 commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1753012656
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
@mchades
I think you can add a matrix parameter about `jdbcBackend` to reduce
repeated line L92.
##
.github/workflows/access-control-integration-test.yml:
##
@@ -22,21 +22,32 @@ jobs:
with:
filters: |
source_changes:
+ - .github/**
- api/**
- authorizations/**
+ - bin/**
Review Comment:
Why do we need to `bin/**` here? for any changes under folder `bin`, I think
`backend test` can cover it.
##
.github/workflows/spark-integration-test.yml:
##
@@ -108,3 +107,64 @@ jobs:
spark-connector/v3.5/spark/build/spark-3.5-integration-test.log
distribution/package/logs/*.out
distribution/package/logs/*.log
+
+ SparkIT-on-pr:
+needs: changes
+if: (github.event_name == 'pull_request' &&
needs.changes.outputs.source_changes == 'true')
Review Comment:
> Is there any way to reuse the code? duplicated code are hard to maintance.
+1. The current situation seems quite complicated to me and may introduce
excessive time to maintain so many CI trigger condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1751829089
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
ok, added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
xunliu commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1751803934
##
.github/workflows/access-control-integration-test.yml:
##
@@ -83,7 +87,8 @@ jobs:
- name: Authorization Integration Test (JDK${{ matrix.java-version
}}-${{ matrix.test-mode }}-${{ matrix.backend }})
id: integrationTest
run: |
- ./gradlew -PskipTests -PtestMode=${{ matrix.test-mode }}
-PjdbcBackend=${{ matrix.backend }} -PjdkVersion=${{ matrix.java-version }}
-PskipDockerTests=false :authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=embedded -PjdbcBackend=h2
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
+ ./gradlew -PskipTests -PtestMode=deploy -PjdbcBackend=mysql
-PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
:authorizations:authorization-ranger:test --tests
"org.apache.gravitino.authorization.ranger.integration.test.**"
Review Comment:
Yes, I think we can run AC IT on PG backend in the deploy mode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
FANNG1 commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1751404237 ## .github/workflows/spark-integration-test.yml: ## @@ -108,3 +107,64 @@ jobs: spark-connector/v3.5/spark/build/spark-3.5-integration-test.log distribution/package/logs/*.out distribution/package/logs/*.log + + SparkIT-on-pr: +needs: changes +if: (github.event_name == 'pull_request' && needs.changes.outputs.source_changes == 'true') Review Comment: Is there any way to reuse the code? duplicated code are hard to maintance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on code in PR #4827: URL: https://github.com/apache/gravitino/pull/4827#discussion_r1751374566 ## .github/workflows/spark-integration-test.yml: ## @@ -108,3 +107,64 @@ jobs: spark-connector/v3.5/spark/build/spark-3.5-integration-test.log distribution/package/logs/*.out distribution/package/logs/*.log + + SparkIT-on-pr: +needs: changes +if: (github.event_name == 'pull_request' && needs.changes.outputs.source_changes == 'true') Review Comment: To decrease the number of SparkCI jobs. Since the 'pull_request' is more frequent than 'push', I reduced the multi-jdk running tests under 'pull_request' and moved them to be tested after PR merging to alleviate CI pressure. Except for the Java version, everything else is the same. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
jerryshao commented on code in PR #4827:
URL: https://github.com/apache/gravitino/pull/4827#discussion_r1750691513
##
.github/workflows/frontend-integration-test.yml:
##
@@ -27,37 +27,33 @@ jobs:
- bin/**
- catalogs/**
- clients/client-java/**
- - clients/client-java-runtime/**
- - clients/filesystem-hadoop3/**
- - clients/filesystem-hadoop3-runtime/**
- common/**
- conf/**
- core/**
- dev/**
- gradle/**
- - integration-test/**
+ - iceberg/**
+ - integration-test-common/**
- meta/**
+ - scripts/**
- server/**
- server-common/**
- - spark-connector/**
- - trino-connector/**
- web/**
- - docs/open-api/**
- build.gradle.kts
- gradle.properties
- gradlew
- setting.gradle.kts
outputs:
source_changes: ${{ steps.filter.outputs.source_changes }}
- # Integration test for AMD64 architecture
- test-amd64-arch:
+ FrontendIT:
needs: changes
if: needs.changes.outputs.source_changes == 'true'
runs-on: ubuntu-latest
timeout-minutes: 60
strategy:
matrix:
+# Integration test for AMD64 architecture
architecture: [linux/amd64]
java-version: [ 8 ]
Review Comment:
Some places we should java 8, some places we use 17, maybe we should unify
them all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4811] improvement(CI): optimize CI jobs trigger conditions [gravitino]
mchades commented on PR #4827: URL: https://github.com/apache/gravitino/pull/4827#issuecomment-2322798468 This PR is based on #4628 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
