Skip to content

Commit

Permalink
[#2292] improvement(PyClient): Fix unstable Python client integration…
Browse files Browse the repository at this point in the history
… test (#2994)

### What changes were proposed in this pull request?

1. Add `skipPyClientITs` param in Gradle build script. Let's Python
client integration test not running in the backend ITs
2. Combined Gradle build command of Python client test and integration
test
3. improvement FilesetCatalog integration test codes.
4. Test mode

+ Test Principle: Every Python ITs class base on the
`IntegrationTestEnv`, `IntegrationTestEnv` will automatic start and stop
Gravitino server to support ITs, But when you run multiple ITs class at
same time, The first test class that finishes running will shut down the
Gravitino server, which will cause other test classes to fail if they
can't connect to the Gravitino server.
+ Run test in the IDE: Through `IntegrationTestEnv` class automatic
start and stop Gravitino server to support ITs.
+ Run test in the Github Action: Manual start and stop Gravitino server,
and set `EXTERNAL_START_GRAVITINO` environment variable
+ Run test in the Gradle command `:client:client-python:test`: Gradle
automatic start and stop Gravitino server, and set
`EXTERNAL_START_GRAVITINO` environment variable.

### Why are the changes needed?

Fix: #2292

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Through CI check.
  • Loading branch information
xunliu authored Apr 18, 2024
1 parent 629c0d3 commit 9e6e189
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 45 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/backend-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ jobs:
- name: Backend Integration Test
id: integrationTest
run: |
./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -PskipWebITs -P${{ matrix.backend }}
./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -PskipWebITs -P${{ matrix.backend }} -PskipPyClientITs
- name: Upload integrate tests reports
uses: actions/upload-artifact@v3
Expand Down
18 changes: 9 additions & 9 deletions .github/workflows/python-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ jobs:
- name: Set up QEMU
uses: docker/setup-qemu-action@v2

- name: Free up disk space
run: |
dev/ci/util_free_space.sh
- name: Python Client Integration Test
id: integrationTest
run: |
#./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }}
#for pythonVersion in "3.8" "3.9" "3.10" "3.11"
#do
# ./gradlew -PjdkVersion=${{ matrix.java-version }} -PpythonVersion=${pythonVersion} :client:client-python:integrationTest
#done
./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }}
for pythonVersion in "3.8" "3.9" "3.10" "3.11"
do
echo "Use Python version ${pythonVersion} to test the Python client."
./gradlew -PjdkVersion=${{ matrix.java-version }} -PpythonVersion=${pythonVersion} :client:client-python:test
# Clean Gravitino database to clean test data
rm -rf ./distribution/package/data
done
- name: Upload integrate tests reports
uses: actions/upload-artifact@v3
Expand Down
39 changes: 19 additions & 20 deletions clients/client-python/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ fun gravitinoServer(operation: String) {
val exitCode = process.waitFor()
if (exitCode == 0) {
val currentContext = process.inputStream.bufferedReader().readText()
println("Current docker context is: $currentContext")
println("Gravitino server status: $currentContext")
} else {
println("checkOrbStackStatus Command execution failed with exit code $exitCode")
println("Gravitino server execution failed with exit code $exitCode")
}
}

Expand All @@ -39,26 +39,25 @@ tasks {
}

val test by registering(VenvTask::class) {
dependsOn(pipInstall)
venvExec = "python"
args = listOf("-m", "unittest")
workingDir = projectDir.resolve(".")
}

val integrationTest by registering(VenvTask::class) {
doFirst() {
gravitinoServer("start")
}
val skipPyClientITs = project.hasProperty("skipPyClientITs")
if (!skipPyClientITs) {
doFirst {
gravitinoServer("start")
}

dependsOn(pipInstall)
venvExec = "python"
args = listOf("-m", "unittest")
workingDir = projectDir.resolve(".")
environment = mapOf("PROJECT_VERSION" to project.version,
"GRADLE_START_GRAVITINO" to "True")
dependsOn(pipInstall)
venvExec = "python"
args = listOf("-m", "unittest")
workingDir = projectDir.resolve(".")
environment = mapOf(
"PROJECT_VERSION" to project.version,
"GRAVITINO_HOME" to project.rootDir.path + "/distribution/package",
"START_EXTERNAL_GRAVITINO" to "true"
)

doLast {
gravitinoServer("stop")
doLast {
gravitinoServer("stop")
}
}
}

Expand Down
24 changes: 14 additions & 10 deletions clients/client-python/tests/integration/integration_test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ def get_gravitino_server_version():
response.close()
return True
except requests.exceptions.RequestException as e:
logger.warning("Failed to access the server: {}", e)
logger.warning("Failed to access the Gravitino server")
return False


def check_gravitino_server_status():
def check_gravitino_server_status() -> bool:
gravitino_server_running = False
for i in range(5):
logger.info("Monitoring Gravitino server status. Attempt %s", i + 1)
Expand Down Expand Up @@ -53,15 +53,21 @@ class IntegrationTestEnv(unittest.TestCase):
def setUpClass(cls):
_init_logging()

if os.environ.get('GRADLE_START_GRAVITINO') is not None:
logger.info('Manual start gravitino server [%s].', check_gravitino_server_status())
if os.environ.get('START_EXTERNAL_GRAVITINO') is not None:
"""Maybe Gravitino server already startup by Gradle test command or developer manual startup."""
if not check_gravitino_server_status():
logger.error("ERROR: Can't find online Gravitino server!")
return

current_path = os.getcwd()
cls.gravitino_startup_script = os.path.join(current_path, '../../../distribution/package/bin/gravitino.sh')
GravitinoHome = os.environ.get('GRAVITINO_HOME')
if GravitinoHome is None:
logger.error('Gravitino Python client integration test must configure `GRAVITINO_HOME`')
quit(0)

cls.gravitino_startup_script = os.path.join(GravitinoHome, 'bin/gravitino.sh')
if not os.path.exists(cls.gravitino_startup_script):
logger.error("Can't find Gravitino startup script: %s, "
"Please execute `./gradlew compileDistribution -x test` in the gravitino project root "
"Please execute `./gradlew compileDistribution -x test` in the Gravitino project root "
"directory.", cls.gravitino_startup_script)
quit(0)

Expand All @@ -78,11 +84,9 @@ def setUpClass(cls):
logger.error("ERROR: Can't start Gravitino server!")
quit(0)

cls.clean_test_date()

@classmethod
def tearDownClass(cls):
if os.environ.get('GRADLE_START_GRAVITINO') is not None:
if os.environ.get('START_EXTERNAL_GRAVITINO') is not None:
return

logger.info("Stop integration test environment...")
Expand Down
11 changes: 6 additions & 5 deletions clients/client-python/tests/integration/test_fileset_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
This software is licensed under the Apache License version 2.
"""
import logging
from random import random, randint

from gravitino.api.catalog import Catalog
from gravitino.api.fileset import Fileset
Expand All @@ -20,11 +21,11 @@
class TestFilesetCatalog(IntegrationTestEnv):
catalog: Catalog = None
metalake: GravitinoMetalake = None
metalake_name: str = "testMetalake"
catalog_name: str = "testCatalog"
schema_name: str = "testSchema"
fileset_name: str = "testFileset1"
fileset_alter_name: str = "testFilesetAlter"
metalake_name: str = "testMetalake" + str(randint(1, 100))
catalog_name: str = "testCatalog" + str(randint(1, 100))
schema_name: str = "testSchema" + str(randint(1, 100))
fileset_name: str = "testFileset1" + str(randint(1, 100))
fileset_alter_name: str = "testFilesetAlter" + str(randint(1, 100))
provider: str = "hadoop"

metalake_ident: NameIdentifier = NameIdentifier.of(metalake_name)
Expand Down

0 comments on commit 9e6e189

Please sign in to comment.