You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "arokoolamayowa (via GitHub)" <gi...@apache.org> on 2025/09/23 09:39:01 UTC

[PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

arokoolamayowa opened a new pull request, #14167:
URL: https://github.com/apache/iceberg/pull/14167

   This PR introduces a simple shell script (test.sh) to verify the build, run, and basic health of the iceberg-rest-fixture Docker image. The script:
   
   - Builds the required JAR using Gradle.
   
   - Builds the Docker image.
   
   - Starts a container and waits for it to become healthy.
   
   - Performs a manual healthcheck and a basic check on the /v1/config endpoint.
   
   - Cleans up resources afterward.
   
   This is intended as a beginner-friendly contribution to improve testing around the Docker fixture. It aligns with the goals in #13229 for enhancing integration testing infrastructure, though it doesn't directly resolve any specific bug.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14167:
URL: https://github.com/apache/iceberg/pull/14167#issuecomment-3539522010

   This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "arokoolamayowa (via GitHub)" <gi...@apache.org>.
arokoolamayowa commented on code in PR #14167:
URL: https://github.com/apache/iceberg/pull/14167#discussion_r2386487890


##########
docker/iceberg-rest-fixture/test.sh:
##########
@@ -0,0 +1,56 @@
+#!/bin/bash
+
+set -euo pipefail  # Exit on error, treat unset variables as error, fail on pipeline errors
+
+# Define variables
+IMAGE_NAME="iceberg-rest-fixture:test"
+DOCKERFILE_PATH="docker/iceberg-rest-fixture/Dockerfile"
+CONTAINER_NAME="iceberg-rest-test"
+PORT=8181
+
+# Step 1: Build the required JAR using Gradle
+echo "Building the open-api module JAR..."
+./gradlew :open-api:build
+
+# Step 2: Build the Docker image
+echo "Building Docker image from $DOCKERFILE_PATH..."
+docker build -t $IMAGE_NAME -f $DOCKERFILE_PATH .
+
+# Step 3: Run the container in detached mode
+echo "Starting container..."
+docker run -d --name $CONTAINER_NAME -p $PORT:$PORT $IMAGE_NAME
+
+# Step 4: Wait for the container to start (healthcheck interval is 1s, retries 10, so wait up to 15s)
+echo "Waiting for healthcheck..."
+sleep 15
+
+# Step 5: Check if the container is running
+if [ "$(docker inspect -f '{{.State.Running}}' $CONTAINER_NAME)" != "true" ]; then
+  echo "Error: Container failed to start."
+  docker logs $CONTAINER_NAME
+  exit 1
+fi
+
+# Step 6: Perform healthcheck manually (matches the Dockerfile's HEALTHCHECK)

Review Comment:
   i have updated the code to do that 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "arokoolamayowa (via GitHub)" <gi...@apache.org>.
arokoolamayowa commented on PR #14167:
URL: https://github.com/apache/iceberg/pull/14167#issuecomment-3369827053

   Hi,
   this PR has been approved by @shangxinli.  
   Could someone with write access please review or approve it for merging?  
   Thanks in advance!


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14167:
URL: https://github.com/apache/iceberg/pull/14167#issuecomment-3507223041

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #14167: Add basic test script for iceberg-rest-fixture Dockerfile 
URL: https://github.com/apache/iceberg/pull/14167


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Add basic test script for iceberg-rest-fixture Dockerfile [iceberg]

Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #14167:
URL: https://github.com/apache/iceberg/pull/14167#discussion_r2379112411


##########
docker/iceberg-rest-fixture/test.sh:
##########
@@ -0,0 +1,56 @@
+#!/bin/bash
+
+set -euo pipefail  # Exit on error, treat unset variables as error, fail on pipeline errors
+
+# Define variables
+IMAGE_NAME="iceberg-rest-fixture:test"
+DOCKERFILE_PATH="docker/iceberg-rest-fixture/Dockerfile"
+CONTAINER_NAME="iceberg-rest-test"
+PORT=8181
+
+# Step 1: Build the required JAR using Gradle
+echo "Building the open-api module JAR..."
+./gradlew :open-api:build
+
+# Step 2: Build the Docker image
+echo "Building Docker image from $DOCKERFILE_PATH..."
+docker build -t $IMAGE_NAME -f $DOCKERFILE_PATH .
+
+# Step 3: Run the container in detached mode
+echo "Starting container..."
+docker run -d --name $CONTAINER_NAME -p $PORT:$PORT $IMAGE_NAME
+
+# Step 4: Wait for the container to start (healthcheck interval is 1s, retries 10, so wait up to 15s)
+echo "Waiting for healthcheck..."
+sleep 15

Review Comment:
   Fixed 15-second sleep is brittle. Consider implementing a proper health check loop



##########
docker/iceberg-rest-fixture/test.sh:
##########
@@ -0,0 +1,56 @@
+#!/bin/bash
+
+set -euo pipefail  # Exit on error, treat unset variables as error, fail on pipeline errors
+
+# Define variables
+IMAGE_NAME="iceberg-rest-fixture:test"
+DOCKERFILE_PATH="docker/iceberg-rest-fixture/Dockerfile"
+CONTAINER_NAME="iceberg-rest-test"
+PORT=8181
+
+# Step 1: Build the required JAR using Gradle
+echo "Building the open-api module JAR..."
+./gradlew :open-api:build
+
+# Step 2: Build the Docker image
+echo "Building Docker image from $DOCKERFILE_PATH..."
+docker build -t $IMAGE_NAME -f $DOCKERFILE_PATH .
+
+# Step 3: Run the container in detached mode
+echo "Starting container..."
+docker run -d --name $CONTAINER_NAME -p $PORT:$PORT $IMAGE_NAME
+
+# Step 4: Wait for the container to start (healthcheck interval is 1s, retries 10, so wait up to 15s)
+echo "Waiting for healthcheck..."
+sleep 15
+
+# Step 5: Check if the container is running
+if [ "$(docker inspect -f '{{.State.Running}}' $CONTAINER_NAME)" != "true" ]; then
+  echo "Error: Container failed to start."
+  docker logs $CONTAINER_NAME
+  exit 1
+fi
+
+# Step 6: Perform healthcheck manually (matches the Dockerfile's HEALTHCHECK)

Review Comment:
   Does this manual healthcheck duplicate the Docker HEALTHCHECK.



##########
docker/iceberg-rest-fixture/test.sh:
##########
@@ -0,0 +1,56 @@
+#!/bin/bash
+
+set -euo pipefail  # Exit on error, treat unset variables as error, fail on pipeline errors
+
+# Define variables
+IMAGE_NAME="iceberg-rest-fixture:test"
+DOCKERFILE_PATH="docker/iceberg-rest-fixture/Dockerfile"
+CONTAINER_NAME="iceberg-rest-test"
+PORT=8181
+
+# Step 1: Build the required JAR using Gradle
+echo "Building the open-api module JAR..."
+./gradlew :open-api:build
+
+# Step 2: Build the Docker image
+echo "Building Docker image from $DOCKERFILE_PATH..."
+docker build -t $IMAGE_NAME -f $DOCKERFILE_PATH .
+
+# Step 3: Run the container in detached mode
+echo "Starting container..."
+docker run -d --name $CONTAINER_NAME -p $PORT:$PORT $IMAGE_NAME

Review Comment:
   Consider adding --quiet flag to reduce build output noise, or add a verbose flag option



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org