-
Notifications
You must be signed in to change notification settings - Fork 74
feat: Implement backend deployment script (#1089) #1107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Implement backend deployment script (#1089) #1107
Conversation
WalkthroughThe changes introduce a new deployment automation script for the backend and update the README to provide a more detailed project description and clarify the use of the Changes
Possibly related PRs
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
README.md (2)
80-82
: Drop the leading$
to satisfy markdownlint and keep examples copy-paste-friendly
MD014
warns against prefixing commands with$
when no output is shown. Earlier lines were already cleaned up, but this one slipped through.-```sh -$ uv sync +```sh +uv sync
7-8
: Minor punctuation tweak for sentence flowThe sentence ends with “for the purposes of -” and then immediately starts a list. A colon is clearer than a hyphen.
-...to communicate with, for the purposes of - +...to communicate with, for the purposes of:scripts/deploy_backend.sh (3)
32-35
: Silence ShellCheck warning for dynamicsource
SC1090
flags variable-based paths. Add an inline directive so CI stays green.-source "$NVM_SH_PATH" +# shellcheck source=/dev/null +source "$NVM_SH_PATH"
40-50
: Usegit pull --ff-only
(or fetch + merge) to avoid unintended mergesA plain
git pull
may create merge commits if the local branch diverged.-git pull origin main +git pull --ff-only origin main
62-66
: Consider--update-env
with PM2 restartsIf environment variables changed,
pm2 restart --update-env
ensures the new values propagate.-"$PM2_PATH" restart django-celery-worker django-backend-asgi +"$PM2_PATH" restart --update-env django-celery-worker django-backend-asgi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md
(3 hunks)scripts/deploy_backend.sh
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: Ishankoradia
PR: DalgoT4D/DDP_backend#1052
File: ddpui/ddpairbyte/airbyte_service.py:739-744
Timestamp: 2025-05-06T09:40:01.263Z
Learning: In the DDP_backend project, field validation for API requests is handled by Pydantic schemas, which automatically validate required fields and return 422 errors when validation fails, making additional manual validation unnecessary.
🪛 markdownlint-cli2 (0.17.2)
README.md
81-81: Dollar signs used before commands without showing output
(MD014, commands-show-output)
🪛 Shellcheck (0.10.0)
scripts/deploy_backend.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 34-34: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
# --- Load Seed Data (if applicable) --- | ||
echo "Loading seed data..." | ||
"$UV_RUN_PATH" run python manage.py loaddata seed/*.json | ||
echo "Seed data loaded." | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard seed-data loading to avoid failing when the directory is empty
If seed/*.json
expands to the literal string (no matches) the command errors in set -e
mode.
-echo "Loading seed data..."
-"$UV_RUN_PATH" run python manage.py loaddata seed/*.json
-echo "Seed data loaded."
+if compgen -G "seed/*.json" >/dev/null; then
+ echo "Loading seed data..."
+ "$UV_RUN_PATH" run python manage.py loaddata seed/*.json
+ echo "Seed data loaded."
+else
+ echo "No seed data found, skipping loaddata."
+fi
🤖 Prompt for AI Agents
In scripts/deploy_backend.sh around lines 57 to 61, the command to load seed
data using "python manage.py loaddata seed/*.json" can fail if the seed
directory is empty because the glob pattern does not match any files. To fix
this, add a guard condition to check if there are any JSON files in the seed
directory before running the loaddata command. This can be done by testing if
the glob expands to existing files and only then executing the load command to
prevent errors in set -e mode.
# This script automates the deployment steps for the DDP_backend application. | ||
# It assumes it's being run on the target EC2 server itself, or via an SSH connection. | ||
# It mirrors the logic currently found in .github/workflows/dalgo-cd.yml | ||
|
||
echo "Starting DDP_backend deployment script..." | ||
|
||
# Ensure script exits immediately if any command fails | ||
set -e | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a proper shebang and safer shell flags
Without a shebang the script’s interpreter is undefined (SC2148
).
Also consider set -o pipefail
to catch failures in piped commands.
-# This script automates the deployment steps for the DDP_backend application.
+#!/usr/bin/env bash
+# This script automates the deployment steps for the DDP_backend application.
+
+# shellcheck disable=SC1090 # NVM path is provided via variable
+
+# Fail fast on any error and on errors inside pipelines
+set -euo pipefail
🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
In scripts/deploy_backend.sh at the beginning (lines 1 to 9), add a proper
shebang line such as `#!/bin/bash` to explicitly define the script interpreter.
Also, update the shell flags by adding `set -o pipefail` along with `set -e` to
ensure the script catches failures in piped commands, making the script
execution safer and more robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ravi-Dagar021199 this looks like a reasonable suggestion to incorporate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
Hi @fatchat , Just wanted to gently ping on this Pull Request (PR #1089 ) for Issue #1015, which focuses on adding type annotations to the Airbyte-related backend files. It's been about three days since it was opened, and I'd appreciate it if someone could take a look when they have a moment. Please let me know if you have any questions or if anything needs clarification on my end. Thanks!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks pretty good
@Ishankoradia can you do a second check please
@Ravi-Dagar021199 would you incorporate the coderabbit suggestions |
This Pull Request addresses the following:
Issue make a deployment script for DDP_backend #1089: "Make a deployment script for DDP_backend":
scripts/deploy_backend.sh
, a standalone shell script that automates the backend deployment steps..github/workflows/dalgo-cd.yml
for staging deployments, including Git pull, Django migrations, seed data loading, and PM2 process restarts.Previous README.md feedback:
test_script.py
as it was an ad-hoc script and not required.I've ensured the script is executable and the changes are reflected in this PR.
Please let me know if any further adjustments are needed.
Summary by CodeRabbit