-
Notifications
You must be signed in to change notification settings - Fork 990
fix(day03): move cd outside if-block, add docker-compose down before deploy #17
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,75 +1,75 @@ | ||||||||||||||||||||
| #!/bin/bash | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Deploy a Django app and handle errors | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Function to clone the Django app code | ||||||||||||||||||||
| code_clone() { | ||||||||||||||||||||
| echo "Cloning the Django app..." | ||||||||||||||||||||
| if [ -d "django-notes-app" ]; then | ||||||||||||||||||||
| echo "The code directory already exists. Skipping clone." | ||||||||||||||||||||
| else | ||||||||||||||||||||
| git clone https://github.com/LondheShubham153/django-notes-app.git || { | ||||||||||||||||||||
| echo "Failed to clone the code." | ||||||||||||||||||||
| return 1 | ||||||||||||||||||||
| } | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| << task | ||||||||||||||||||||
| deploy django app | ||||||||||||||||||||
| and handle errors | ||||||||||||||||||||
| task | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| #Function to clone the django app | ||||||||||||||||||||
| code_clone(){ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| echo "Cloning the Django app..." | ||||||||||||||||||||
| git clone https://github.com/LondheShubham153/django-notes-app.git | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+11
to
15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fragile error handling: clone failure assumed to mean directory exists.
Consider checking explicitly: Proposed fix code_clone(){
-
echo "Cloning the Django app..."
- git clone https://github.com/LondheShubham153/django-notes-app.git
+ if [ -d "django-notes-app" ]; then
+ echo "Directory already exists, skipping clone..."
+ else
+ git clone https://github.com/LondheShubham153/django-notes-app.git || { echo "Git clone failed"; return 1; }
+ fi
}Then simplify the call site: -if ! code_clone;
-then
- echo "directory already exists, skipping cloning...."
-fi
+if ! code_clone; then
+ echo "Failed to clone repository"
+ exit 1
+fiAlso applies to: 46-49 🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| # Function to install required dependencies | ||||||||||||||||||||
| install_requirements() { | ||||||||||||||||||||
| echo "Installing dependencies..." | ||||||||||||||||||||
| sudo apt-get update && sudo apt-get install -y docker.io nginx docker-compose || { | ||||||||||||||||||||
| echo "Failed to install dependencies." | ||||||||||||||||||||
| return 1 | ||||||||||||||||||||
| } | ||||||||||||||||||||
| #Function to install required dependencies | ||||||||||||||||||||
| install_req() { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| echo " Installing dependencies..." | ||||||||||||||||||||
| sudo apt-get install docker.io nginx -y docker-compose | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+18
to
22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing The function omits Proposed fix install_req() {
-
echo " Installing dependencies..."
- sudo apt-get install docker.io nginx -y docker-compose
+ sudo apt-get update && sudo apt-get install -y docker.io nginx docker-compose
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| # Function to perform required restarts | ||||||||||||||||||||
| required_restarts() { | ||||||||||||||||||||
| echo "Performing required restarts..." | ||||||||||||||||||||
| sudo chown "$USER" /var/run/docker.sock || { | ||||||||||||||||||||
| echo "Failed to change ownership of docker.sock." | ||||||||||||||||||||
| return 1 | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Uncomment the following lines if needed: | ||||||||||||||||||||
| # sudo systemctl enable docker | ||||||||||||||||||||
| # sudo systemctl enable nginx | ||||||||||||||||||||
| # sudo systemctl restart docker | ||||||||||||||||||||
| #Function to perform required restarts | ||||||||||||||||||||
| req_restarts() { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| echo "Performing required restarts" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| sudo chown $USER /var/run/docker.sock | ||||||||||||||||||||
| #sudo systemctl enable docker | ||||||||||||||||||||
| #sudo systemctl enable nginx | ||||||||||||||||||||
| #sudo systemctl restart docker | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Function to deploy the Django app | ||||||||||||||||||||
| deploy() { | ||||||||||||||||||||
| echo "Building and deploying the Django app..." | ||||||||||||||||||||
| docker build -t notes-app . && docker-compose up -d || { | ||||||||||||||||||||
| echo "Failed to build and deploy the app." | ||||||||||||||||||||
| return 1 | ||||||||||||||||||||
| } | ||||||||||||||||||||
| #Fucntion to deploy Django app | ||||||||||||||||||||
| deploy(){ | ||||||||||||||||||||
| echo "Building and Deploying Django app..." #Fixed deployment : Old networks from previous runs had different settings, causing the new deployment to crash. | ||||||||||||||||||||
| #Added docker-compose down to the start of the deploy function to wipe the slate clean. | ||||||||||||||||||||
| docker-compose down | ||||||||||||||||||||
| docker build -t notes-app . | ||||||||||||||||||||
| docker-compose up -d | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Main deployment script | ||||||||||||||||||||
| echo "********** DEPLOYMENT STARTED *********" | ||||||||||||||||||||
| echo "***************** DEPLOYMENT STARTED ******************" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Clone the code | ||||||||||||||||||||
| if ! code_clone; then | ||||||||||||||||||||
| cd django-notes-app || exit 1 | ||||||||||||||||||||
| if ! code_clone; | ||||||||||||||||||||
| then | ||||||||||||||||||||
| echo "directory already exists, skipping cloning...." | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Install dependencies | ||||||||||||||||||||
| if ! install_requirements; then | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| #Moved cd outside the logic so it always enters the app folder before building. | ||||||||||||||||||||
| cd django-notes-app || { echo "Failed to enter directory" ; exit 1; } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| if ! install_req; | ||||||||||||||||||||
| then | ||||||||||||||||||||
| echo "Failed to install requirements" | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Perform required restarts | ||||||||||||||||||||
| if ! required_restarts; then | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if ! req_restarts; | ||||||||||||||||||||
| then | ||||||||||||||||||||
| echo "System fault identitfied" | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: "identitfied" should be "identified". Proposed fix- echo "System fault identitfied"
+ echo "System fault identified"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Deploy the app | ||||||||||||||||||||
| if ! deploy; then | ||||||||||||||||||||
| echo "Deployment failed. Mailing the admin..." | ||||||||||||||||||||
| # Add your sendmail or notification logic here | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| if ! deploy; | ||||||||||||||||||||
| then | ||||||||||||||||||||
| echo "Deployment Failed" | ||||||||||||||||||||
| # sendmail | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| echo "********** DEPLOYMENT DONE *********" | ||||||||||||||||||||
| echo "***************** DEPLOYMENT DONE ******************" | ||||||||||||||||||||
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.
Heredoc used without a command triggers Shellcheck warning SC2188.
The
<< taskheredoc is used as a comment but has no command to consume it. Either prefix with: <<(no-op) or use standard#comments.Proposed fix
Or use a no-op heredoc:
📝 Committable suggestion
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 4-4: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).
(SC2188)
🤖 Prompt for AI Agents