Copilot commented on code in PR #9998:
URL: https://github.com/apache/gravitino/pull/9998#discussion_r2803180718
##########
.github/workflows/build.yml:
##########
@@ -143,7 +143,44 @@ jobs:
- name: Build with Gradle (skip mcp-server tests)
if: needs.changes.outputs.mcp_server_changes != 'true'
- run: ./gradlew build -PskipITs -PskipDockerTests=false -x
:clients:client-python:build -x :mcp-server:build -x :mcp-server:test -x
:mcp-server:pylint
+ run: >
+ ./gradlew build
+ -PskipITs
+ -PskipDockerTests=false
+ -x :clients:client-python:build
+ -x :mcp-server:build
+ -x :mcp-server:test
+ -x :mcp-server:pylint
+ -x :catalogs:catalog-jdbc-clickhouse:build
+ -x :catalogs:catalog-jdbc-clickhouse:test
+ -x :catalogs:catalog-jdbc-mysql:build
+ -x :catalogs:catalog-jdbc-mysql:test
+ -x :catalogs:catalog-jdbc-postgresql:build
+ -x :catalogs:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-fileset:build
+ -x :catalogs-contrib:catalog-fileset:test
+ -x :catalogs-contrib:catalog-hive:build
+ -x :catalogs-contrib:catalog-hive:test
+ -x :catalogs-contrib:catalog-jdbc-mysql:build
+ -x :catalogs-contrib:catalog-jdbc-mysql:test
+ -x :catalogs-contrib:catalog-jdbc-postgresql:build
+ -x :catalogs-contrib:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-kafka:build
+ -x :catalogs-contrib:catalog-kafka:test
+ -x :catalogs-contrib:catalog-lakehouse-generic:build
+ -x :catalogs-contrib:catalog-lakehouse-generic:test
+ -x :catalogs-contrib:catalog-lakehouse-hudi:build
+ -x :catalogs-contrib:catalog-lakehouse-hudi:test
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:build
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:test
+ -x :catalogs-contrib:catalog-lakehouse-paimon:build
+ -x :catalogs-contrib:catalog-lakehouse-paimon:test
+ -x :catalogs-contrib:catalog-model:build
+ -x :catalogs-contrib:catalog-model:test
+ -x :catalogs-contrib:hive-metastore2-libs:build
+ -x :catalogs-contrib:hive-metastore2-libs:test
+ -x :catalogs-contrib:hive-metastore3-libs:build
+ -x :catalogs-contrib:hive-metastore3-libs:test
Review Comment:
This PR has no meaningful title or description. The title "[TEST]" and the
empty template fields provide no context about what changes are being made or
why.
For a PR that:
1. Upgrades testcontainers dependency
2. Adds OceanBase memory configuration
3. Excludes numerous catalog modules from CI builds
4. Modifies artifact upload patterns
A proper description is essential to explain the motivation, testing
approach, and whether these changes are intended for production or are
experimental/debugging changes.
##########
.github/workflows/build.yml:
##########
@@ -143,7 +143,44 @@ jobs:
- name: Build with Gradle (skip mcp-server tests)
if: needs.changes.outputs.mcp_server_changes != 'true'
- run: ./gradlew build -PskipITs -PskipDockerTests=false -x
:clients:client-python:build -x :mcp-server:build -x :mcp-server:test -x
:mcp-server:pylint
+ run: >
+ ./gradlew build
+ -PskipITs
+ -PskipDockerTests=false
+ -x :clients:client-python:build
+ -x :mcp-server:build
+ -x :mcp-server:test
+ -x :mcp-server:pylint
+ -x :catalogs:catalog-jdbc-clickhouse:build
+ -x :catalogs:catalog-jdbc-clickhouse:test
+ -x :catalogs:catalog-jdbc-mysql:build
+ -x :catalogs:catalog-jdbc-mysql:test
+ -x :catalogs:catalog-jdbc-postgresql:build
+ -x :catalogs:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-fileset:build
+ -x :catalogs-contrib:catalog-fileset:test
+ -x :catalogs-contrib:catalog-hive:build
+ -x :catalogs-contrib:catalog-hive:test
+ -x :catalogs-contrib:catalog-jdbc-mysql:build
+ -x :catalogs-contrib:catalog-jdbc-mysql:test
+ -x :catalogs-contrib:catalog-jdbc-postgresql:build
+ -x :catalogs-contrib:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-kafka:build
+ -x :catalogs-contrib:catalog-kafka:test
+ -x :catalogs-contrib:catalog-lakehouse-generic:build
+ -x :catalogs-contrib:catalog-lakehouse-generic:test
+ -x :catalogs-contrib:catalog-lakehouse-hudi:build
+ -x :catalogs-contrib:catalog-lakehouse-hudi:test
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:build
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:test
+ -x :catalogs-contrib:catalog-lakehouse-paimon:build
+ -x :catalogs-contrib:catalog-lakehouse-paimon:test
+ -x :catalogs-contrib:catalog-model:build
+ -x :catalogs-contrib:catalog-model:test
+ -x :catalogs-contrib:hive-metastore2-libs:build
+ -x :catalogs-contrib:hive-metastore2-libs:test
+ -x :catalogs-contrib:hive-metastore3-libs:build
+ -x :catalogs-contrib:hive-metastore3-libs:test
Review Comment:
This PR excludes a large number of catalog modules from the build. This
change significantly reduces CI test coverage by skipping both build and test
tasks for many critical catalog components including ClickHouse, MySQL,
PostgreSQL, Hive, Kafka, and all lakehouse catalogs (Iceberg, Hudi, Paimon,
etc.).
This appears to be a temporary debugging change rather than an intentional
production change. If this is meant to be temporary, it should not be merged.
If there are legitimate reasons for excluding these modules (e.g., fixing flaky
tests, resource constraints), those should be documented in the PR description
and likely handled differently (perhaps through conditional execution or
separate CI jobs).
```suggestion
```
##########
.github/workflows/build.yml:
##########
@@ -143,7 +143,44 @@ jobs:
- name: Build with Gradle (skip mcp-server tests)
if: needs.changes.outputs.mcp_server_changes != 'true'
- run: ./gradlew build -PskipITs -PskipDockerTests=false -x
:clients:client-python:build -x :mcp-server:build -x :mcp-server:test -x
:mcp-server:pylint
+ run: >
+ ./gradlew build
+ -PskipITs
+ -PskipDockerTests=false
+ -x :clients:client-python:build
+ -x :mcp-server:build
+ -x :mcp-server:test
+ -x :mcp-server:pylint
+ -x :catalogs:catalog-jdbc-clickhouse:build
+ -x :catalogs:catalog-jdbc-clickhouse:test
Review Comment:
This exclusion references `:catalogs:catalog-jdbc-clickhouse:build` and
`:catalogs:catalog-jdbc-clickhouse:test`, but this module does not exist in the
`catalogs/` directory. The ClickHouse catalog is actually located at
`catalogs-contrib/catalog-jdbc-clickhouse/`. This incorrect path means the
exclusion has no effect, but it indicates confusion about the module structure.
```suggestion
-x :catalogs-contrib:catalog-jdbc-clickhouse:build
-x :catalogs-contrib:catalog-jdbc-clickhouse:test
```
##########
.github/workflows/build.yml:
##########
@@ -143,7 +143,44 @@ jobs:
- name: Build with Gradle (skip mcp-server tests)
if: needs.changes.outputs.mcp_server_changes != 'true'
- run: ./gradlew build -PskipITs -PskipDockerTests=false -x
:clients:client-python:build -x :mcp-server:build -x :mcp-server:test -x
:mcp-server:pylint
+ run: >
+ ./gradlew build
+ -PskipITs
+ -PskipDockerTests=false
+ -x :clients:client-python:build
+ -x :mcp-server:build
+ -x :mcp-server:test
+ -x :mcp-server:pylint
+ -x :catalogs:catalog-jdbc-clickhouse:build
+ -x :catalogs:catalog-jdbc-clickhouse:test
+ -x :catalogs:catalog-jdbc-mysql:build
+ -x :catalogs:catalog-jdbc-mysql:test
+ -x :catalogs:catalog-jdbc-postgresql:build
+ -x :catalogs:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-fileset:build
+ -x :catalogs-contrib:catalog-fileset:test
+ -x :catalogs-contrib:catalog-hive:build
+ -x :catalogs-contrib:catalog-hive:test
+ -x :catalogs-contrib:catalog-jdbc-mysql:build
+ -x :catalogs-contrib:catalog-jdbc-mysql:test
+ -x :catalogs-contrib:catalog-jdbc-postgresql:build
+ -x :catalogs-contrib:catalog-jdbc-postgresql:test
+ -x :catalogs-contrib:catalog-kafka:build
+ -x :catalogs-contrib:catalog-kafka:test
+ -x :catalogs-contrib:catalog-lakehouse-generic:build
+ -x :catalogs-contrib:catalog-lakehouse-generic:test
+ -x :catalogs-contrib:catalog-lakehouse-hudi:build
+ -x :catalogs-contrib:catalog-lakehouse-hudi:test
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:build
+ -x :catalogs-contrib:catalog-lakehouse-iceberg:test
+ -x :catalogs-contrib:catalog-lakehouse-paimon:build
+ -x :catalogs-contrib:catalog-lakehouse-paimon:test
+ -x :catalogs-contrib:catalog-model:build
+ -x :catalogs-contrib:catalog-model:test
+ -x :catalogs-contrib:hive-metastore2-libs:build
+ -x :catalogs-contrib:hive-metastore2-libs:test
+ -x :catalogs-contrib:hive-metastore3-libs:build
+ -x :catalogs-contrib:hive-metastore3-libs:test
Review Comment:
Many of the excluded module paths are incorrect. For example:
- `:catalogs-contrib:catalog-fileset:build` - but catalog-fileset is in
`catalogs/`, not `catalogs-contrib/`
- `:catalogs-contrib:catalog-hive:build` - but catalog-hive is in
`catalogs/`, not `catalogs-contrib/`
- `:catalogs-contrib:catalog-kafka:build` - but catalog-kafka is in
`catalogs/`, not `catalogs-contrib/`
- And many others...
The `catalogs-contrib/` directory only contains `catalog-jdbc-clickhouse`
and `catalog-jdbc-oceanbase`. Most of the excluded modules with
`catalogs-contrib:` prefix don't actually exist at those paths, so these
exclusions have no effect. This indicates a misunderstanding of the project
structure.
```suggestion
-x :catalogs:catalog-fileset:build
-x :catalogs:catalog-fileset:test
-x :catalogs:catalog-hive:build
-x :catalogs:catalog-hive:test
-x :catalogs:catalog-jdbc-mysql:build
-x :catalogs:catalog-jdbc-mysql:test
-x :catalogs:catalog-jdbc-postgresql:build
-x :catalogs:catalog-jdbc-postgresql:test
-x :catalogs:catalog-kafka:build
-x :catalogs:catalog-kafka:test
-x :catalogs:catalog-lakehouse-generic:build
-x :catalogs:catalog-lakehouse-generic:test
-x :catalogs:catalog-lakehouse-hudi:build
-x :catalogs:catalog-lakehouse-hudi:test
-x :catalogs:catalog-lakehouse-iceberg:build
-x :catalogs:catalog-lakehouse-iceberg:test
-x :catalogs:catalog-lakehouse-paimon:build
-x :catalogs:catalog-lakehouse-paimon:test
-x :catalogs:catalog-model:build
-x :catalogs:catalog-model:test
-x :catalogs:hive-metastore2-libs:build
-x :catalogs:hive-metastore2-libs:test
-x :catalogs:hive-metastore3-libs:build
-x :catalogs:hive-metastore3-libs: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]
