Auto Submission Of Flink Job#156
Auto Submission Of Flink Job#156prashanthShiksha wants to merge 1 commit intoELEVATE-Project:release-3.1.0from
Conversation
📝 WalkthroughWalkthroughThe changes modify Docker Compose configuration to use a new entrypoint script for the jobmanager container that orchestrates service startup and job submission. The elevate-data service is simplified by pinning its image to version v3.1.0 and removing custom volume mounts and entrypoint overrides. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Documentation/Docker-setup/docker-compose.yml`:
- Around line 48-55: The docker-compose command references
/jobmanager-entrypoint.sh which may not be executable when bind-mounted; either
ensure the repository's Documentation/Docker-setup/jobmanager-entrypoint.sh has
executable mode set in git (chmod +x and commit) or change the service command
to invoke the script via an explicit shell (e.g., bash
/jobmanager-entrypoint.sh) so container startup doesn't fail; update the command
entry for the jobmanager service accordingly and verify the tracked file mode
for jobmanager-entrypoint.sh.
In `@Documentation/Docker-setup/jobmanager-entrypoint.sh`:
- Around line 10-20: Remove the duplicate "Waiting for JobManager to be
ready..." echo and make the readiness loop deterministic by adding an explicit
timeout failure path: keep the for loop that checks /dev/tcp on port 8081, but
after the loop verify whether the service became ready (i.e., whether
"JobManager is up!" was reached) and if not print a clear error like "JobManager
failed to become ready after timeout" and exit with a non-zero status (exit 1)
so callers can detect failure; update the loop messages accordingly so only one
initial waiting message exists and a success or failure message is printed.
| command: /jobmanager-entrypoint.sh | ||
| environment: | ||
| - | | ||
| FLINK_PROPERTIES= | ||
| jobmanager.rpc.address: jobmanager | ||
| volumes: | ||
| - ./Documentation/Docker-setup/jobmanager-entrypoint.sh:/jobmanager-entrypoint.sh | ||
| - ./stream-jobs/combined-stream-processor/target/combined-stream-processor-1.0.0.jar:/job-jars/combined-stream-processor-1.0.0.jar |
There was a problem hiding this comment.
Ensure the entrypoint script is executable (or invoke via bash).
When bind-mounting a script, host file permissions apply. If the script isn’t executable, the container fails to start. To make this robust, either ensure the file mode is executable in the repo or invoke it via bash.
🔧 Safer command form
- command: /jobmanager-entrypoint.sh
+ command: ["bash", "/jobmanager-entrypoint.sh"]Please verify the file mode in the repo:
#!/bin/bash
# Check tracked file mode and current permissions
git ls-files -s Documentation/Docker-setup/jobmanager-entrypoint.sh
stat -c '%a %n' Documentation/Docker-setup/jobmanager-entrypoint.sh🤖 Prompt for AI Agents
In `@Documentation/Docker-setup/docker-compose.yml` around lines 48 - 55, The
docker-compose command references /jobmanager-entrypoint.sh which may not be
executable when bind-mounted; either ensure the repository's
Documentation/Docker-setup/jobmanager-entrypoint.sh has executable mode set in
git (chmod +x and commit) or change the service command to invoke the script via
an explicit shell (e.g., bash /jobmanager-entrypoint.sh) so container startup
doesn't fail; update the command entry for the jobmanager service accordingly
and verify the tracked file mode for jobmanager-entrypoint.sh.
There was a problem hiding this comment.
@prashanthShiksha - Change the file mode to executable
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
yes it is executable .
-rwxrwxr-x 1 user2 user2 1614 Jan 19 19:19 jobmanager-entrypoint.sh
| echo "Waiting for JobManager to be ready..." | ||
| # Wait for JobManager to be ready | ||
| echo "Waiting for JobManager to be ready..." | ||
| for i in {1..30}; do | ||
| if (echo > /dev/tcp/localhost/8081) >/dev/null 2>&1; then | ||
| echo "JobManager is up!" | ||
| break | ||
| fi | ||
| echo "Waiting..." | ||
| sleep 2 | ||
| done |
There was a problem hiding this comment.
Handle readiness timeout (and remove the duplicate log line).
Line 10 and Line 12 print the same message, and the loop proceeds even if JobManager never becomes ready. Add an explicit timeout check so failures are clear and deterministic.
💡 Proposed fix
-# Wait for JobManager to be ready
-echo "Waiting for JobManager to be ready..."
-# Wait for JobManager to be ready
-echo "Waiting for JobManager to be ready..."
-for i in {1..30}; do
+echo "Waiting for JobManager to be ready..."
+ready=false
+for i in {1..30}; do
if (echo > /dev/tcp/localhost/8081) >/dev/null 2>&1; then
echo "JobManager is up!"
+ ready=true
break
fi
- echo "Waiting..."
+ echo "Waiting... ($i/30)"
sleep 2
done
+if [ "$ready" != "true" ]; then
+ echo "JobManager did not become ready after 60s; exiting."
+ kill "$JM_PID" || true
+ exit 1
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Waiting for JobManager to be ready..." | |
| # Wait for JobManager to be ready | |
| echo "Waiting for JobManager to be ready..." | |
| for i in {1..30}; do | |
| if (echo > /dev/tcp/localhost/8081) >/dev/null 2>&1; then | |
| echo "JobManager is up!" | |
| break | |
| fi | |
| echo "Waiting..." | |
| sleep 2 | |
| done | |
| echo "Waiting for JobManager to be ready..." | |
| ready=false | |
| for i in {1..30}; do | |
| if (echo > /dev/tcp/localhost/8081) >/dev/null 2>&1; then | |
| echo "JobManager is up!" | |
| ready=true | |
| break | |
| fi | |
| echo "Waiting... ($i/30)" | |
| sleep 2 | |
| done | |
| if [ "$ready" != "true" ]; then | |
| echo "JobManager did not become ready after 60s; exiting." | |
| kill "$JM_PID" || true | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 13-13: i appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In `@Documentation/Docker-setup/jobmanager-entrypoint.sh` around lines 10 - 20,
Remove the duplicate "Waiting for JobManager to be ready..." echo and make the
readiness loop deterministic by adding an explicit timeout failure path: keep
the for loop that checks /dev/tcp on port 8081, but after the loop verify
whether the service became ready (i.e., whether "JobManager is up!" was reached)
and if not print a clear error like "JobManager failed to become ready after
timeout" and exit with a non-zero status (exit 1) so callers can detect failure;
update the loop messages accordingly so only one initial waiting message exists
and a success or failure message is printed.
There was a problem hiding this comment.
removed the duplicate log line
There was a problem hiding this comment.
@prashanthShiksha, thank you for removing the duplicate log line!
Just a gentle reminder that the review also suggested adding explicit timeout handling after the loop to ensure the script exits with a clear error message if JobManager doesn't become ready within the 60-second window. This would make failures more deterministic and easier to debug.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| - ./config_files/config.ini:/app/Documentation/batch-scripts/config.ini | ||
| - ./logs:/app/logs |
There was a problem hiding this comment.
These 2 lines are required, right?
| - ./config_files/metabase-mentoring-dashboard.conf:/app/metabase-jobs/mentoring-dashboard-creator/src/main/resources/metabase-mentoring-dashboard.conf | ||
| - ./config_files/config.ini:/app/Documentation/batch-scripts/config.ini | ||
| - ./logs:/app/logs | ||
| entrypoint: ["/bin/bash", "-c", "/app/Documentation/Docker-setup/submit-jobs.sh jobmanager && sleep infinity"] |
There was a problem hiding this comment.
Also, if we are not using submit-jobs.sh can we remove the file?
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.