jerryshao commented on code in PR #4651:
URL: https://github.com/apache/gravitino/pull/4651#discussion_r1730598765
##########
.github/workflows/authorization-integration-test.yml:
##########
@@ -0,0 +1,110 @@
+name: Authorization Integration Test
+
+# Controls when the workflow will run
+on:
+ # Triggers the workflow on push or pull request events but only for the
"main" branch
+ push:
+ branches: [ "main", "branch-*" ]
+ pull_request:
+ branches: [ "main", "branch-*" ]
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number ||
github.ref }}
+ cancel-in-progress: true
+
+jobs:
+ changes:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v3
+ - uses: dorny/paths-filter@v2
+ id: filter
+ with:
+ filters: |
+ source_changes:
+ - .github/**
+ - api/**
+ - authorizations/**
+ - bin/**
+ - catalogs/**
+ - clients/client-java/**
+ - clients/client-java-runtime/**
+ - clients/filesystem-hadoop3/**
+ - clients/filesystem-hadoop3-runtime/**
+ - common/**
+ - conf/**
+ - core/**
+ - gradle/**
+ - integration-test-common/**
+ - iceberg/**
+ - meta/**
+ - server/**
+ - server-common/**
+ - trino-connector/**
+ - web/**
+ - 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:
+ 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, 11, 17 ]
+ test-mode: [ embedded, deploy ]
Review Comment:
I'm afraid there are so many pipelines that ASF will not allow us to run so
many runners. I would suggest we don't split them so many, maybe one
"java-version" is enough, what do you think?
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -802,25 +812,60 @@ private String buildConfPath(Map<String, String>
properties, String provider) {
return confPath;
}
- private String buildPkgPath(Map<String, String> conf, String provider) {
+ private String buildPkgPath(
+ Map<String, String> conf, String provider,
IsolatedClassLoader.IsolatedType isolatedType) {
String gravitinoHome = System.getenv("GRAVITINO_HOME");
Preconditions.checkArgument(gravitinoHome != null, "GRAVITINO_HOME not
set");
boolean testEnv = System.getenv("GRAVITINO_TEST") != null;
String pkg = conf.get(Catalog.PROPERTY_PACKAGE);
- String pkgPath;
if (pkg != null) {
- pkgPath = String.join(File.separator, pkg, "libs");
- } else if (testEnv) {
- // In test, the catalog package is under the build directory.
- pkgPath =
- String.join(
- File.separator, gravitinoHome, "catalogs", "catalog-" +
provider, "build", "libs");
- } else {
- // In real environment, the catalog package is under the catalog
directory.
- pkgPath = String.join(File.separator, gravitinoHome, "catalogs",
provider, "libs");
+ return String.join(File.separator, pkg, "libs");
Review Comment:
How do we handle this case if `Catalog.PROPERTY_PACKAGE` is set? From the
current code, if `Catalog.PROPERTY_PACKAGE` is set, then we'll directly return
without handling authorization related packages.
##########
.github/workflows/authorization-integration-test.yml:
##########
@@ -0,0 +1,110 @@
+name: Authorization Integration Test
+
+# Controls when the workflow will run
+on:
+ # Triggers the workflow on push or pull request events but only for the
"main" branch
+ push:
+ branches: [ "main", "branch-*" ]
+ pull_request:
+ branches: [ "main", "branch-*" ]
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number ||
github.ref }}
+ cancel-in-progress: true
+
+jobs:
+ changes:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v3
+ - uses: dorny/paths-filter@v2
+ id: filter
+ with:
+ filters: |
+ source_changes:
+ - .github/**
+ - api/**
+ - authorizations/**
+ - bin/**
+ - catalogs/**
+ - clients/client-java/**
+ - clients/client-java-runtime/**
+ - clients/filesystem-hadoop3/**
+ - clients/filesystem-hadoop3-runtime/**
+ - common/**
+ - conf/**
+ - core/**
+ - gradle/**
+ - integration-test-common/**
+ - iceberg/**
+ - meta/**
+ - server/**
+ - server-common/**
+ - trino-connector/**
+ - web/**
+ - build.gradle.kts
+ - gradle.properties
+ - gradlew
+ - setting.gradle.kts
Review Comment:
I think maybe only the code changes in catalog, core, and some other places
is enough to trigger this test, some parts like client, web, etc maybe
unrelated.
##########
gradle/libs.versions.toml:
##########
@@ -79,6 +79,7 @@ flink = "1.18.0"
cglib = "2.2"
ranger = "2.4.0"
javax-jaxb-api = "2.3.1"
+javax-ws-rs-api = "2.1.1"
Review Comment:
Why do we need to add this library? I don't find there's a place using 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]
