Skip to content

Conversation

@moseszane168
Copy link

Optimize CI.

@liubao68
Copy link

Seems nothong changed but pool code structure than before.

@liubao68 liubao68 requested a review from Copilot May 15, 2025 03:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes the CI by simplifying the database initialization script and enhancing the workflow with readiness checks before running tests.

  • Removed retry loop and added direct password replacement and init commands in initialize_database.sh
  • Introduced a “Wait for database” step in the CI workflow
  • Added a pre-test connection verification step

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ci/initialize_database.sh Simplified DB init script: removed retry logic, added sed password replacement and direct docker exec commands
.github/workflows/ci.yml Added a readiness loop before initialization and a pre-test connection check
Comments suppressed due to low confidence (2)

ci/initialize_database.sh:23

  • [nitpick] Using sudo in a CI script can fail if the runner lacks sudo privileges; consider running sed as the current user or ensuring the script has the necessary permissions without relying on sudo.
sudo sed -i "s/'{{OPENGAUSS_PASSWORD}}'/'${password}'/" "${mounted_dir}/${sql_file}"

ci/initialize_database.sh:26

  • [nitpick] The nested quoting for the docker exec command is complex and may break if the file path contains spaces. Consider extracting the SQL file path into a variable and simplifying the quoting or using a here-document for clarity.
docker exec -i ${TTY_FLAG} "${container_name}" bash -c "su - omm -c 'gsql -U omm -c \"CREATE DATABASE gaussdbgo_test;\" -f ${mounted_dir}/${sql_file}'"

run: |
echo "Waiting for database to be fully ready..."
for i in {1..30}; do
if docker exec ${CONTAINER_NAME} bash -c "su - omm -c 'gsql -U omm -c \"select 1;\"'"; then
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readiness check omits specifying the database port and password flags, which may cause failures if the database uses a non-default port or requires authentication. Consider adding -p ${PORT} -W${OPENGAUSS_PASSWORD} to the gsql command for consistency with later steps.

Suggested change
if docker exec ${CONTAINER_NAME} bash -c "su - omm -c 'gsql -U omm -c \"select 1;\"'"; then
if docker exec ${CONTAINER_NAME} bash -c "su - omm -c 'gsql -U omm -p ${PORT} -W${OPENGAUSS_PASSWORD} -c \"select 1;\"'"; then

Copilot uses AI. Check for mistakes.
@liubao68 liubao68 merged commit 1ca5457 into HuaweiCloudDeveloper:master May 15, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants