Skip to content
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.

Sign up for GitHub

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

feat: Update Heroku deployment for Celery and Redis integration #2600

Closed
wants to merge 2 commits into from
Closed

feat: Update Heroku deployment for Celery and Redis integration #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17bfae0
Update Heroku deployment for Celery and Redis integration
areebahmeddd Nov 15, 2024
0bfd424
Added coderrabbitai critical suggestions
areebahmeddd Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Procfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
web: gunicorn config.wsgi:application
release: python manage.py collectstatic --noinput && python manage.py migrate
worker: celery -A config.celery_app worker --loglevel=info
beat: celery -A config.celery_app beat --loglevel=info
91 changes: 64 additions & 27 deletions app.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,64 @@
{
"environments": {
"test": {
"scripts": {
"test": "python manage.py test"
},
"buildpacks": [{
"url": "https://github.com/cyberdelia/heroku-geo-buildpack.git"
},
{
"url": "heroku/python"
}
],
"env": {
"DJANGO_SETTINGS_MODULE": {
"value": "config.settings.test"
},
"LD_LIBRARY_PATH": {
"value": "/app/lib"
},
"BUILD_WITH_GEO_LIBRARIES": {
"value": "1"
}
}
}
}
}
{
"environments": {
"test": {
"scripts": {
"test": "python manage.py test"
},
"buildpacks": [
{
"url": "https://github.com/cyberdelia/heroku-geo-buildpack.git"
},
{
"url": "heroku/python"
}
],
Comment on lines +7 to +14
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Oh, I see you've forgotten something rather important in production...

The production environment is missing the buildpack configurations that are present in the test environment. If your application uses geographical features (which it seems to, given the geo buildpack in test), this could lead to some... interesting behavior in production.

Add the buildpacks to the production environment:

 "production": {
     "scripts": {
         "postdeploy": "python manage.py migrate && python manage.py load_redis_index"
     },
+    "buildpacks": [
+        {
+            "url": "https://github.com/cyberdelia/heroku-geo-buildpack.git"
+        },
+        {
+            "url": "heroku/python"
+        }
+    ],
     "env": {

Also applies to: 27-30

"env": {
"DJANGO_SETTINGS_MODULE": {
"value": "config.settings.test"
},
"LD_LIBRARY_PATH": {
"value": "/app/lib"
},
"BUILD_WITH_GEO_LIBRARIES": {
"value": "1"
}
}
},
"production": {
"scripts": {
"postdeploy": "python manage.py migrate --check && python manage.py migrate && python manage.py load_redis_index || echo 'Post-deploy failed' && exit 1"
},
"buildpacks": [
{
"url": "https://github.com/cyberdelia/heroku-geo-buildpack.git"
},
{
"url": "heroku/python"
}
],
"env": {
"DJANGO_SETTINGS_MODULE": {
"value": "config.settings.production"
},
"DATABASE_URL": {
"value": "${DATABASE_URL}",
"required": true
},
"REDIS_URL": {
"value": "${REDIS_URL}",
"required": true
},
"CELERY_BROKER_URL": {
"value": "${CELERY_BROKER_URL}",
"required": true
},
"LD_LIBRARY_PATH": {
"value": "/app/lib"
},
"BUILD_WITH_GEO_LIBRARIES": {
"value": "1"
}
}
}
}
}
42 changes: 42 additions & 0 deletions docs/local-setup/configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,45 @@ If you get an :code:`ImproperlyConfigured` error regarding the Spatialite librar
::

$ sudo apt install libsqlite3-mod-spatialite

Heroku Deployment
=================

To deploy the backend along with Celery on Heroku, follow these steps:

1. Ensure you have the Heroku CLI installed and are logged in.

2. Create a new Heroku app:

.. code-block:: bash

heroku create your-app-name

3. Add the necessary add-ons for PostgreSQL and Redis:

.. code-block:: bash

heroku addons:create heroku-postgresql:hobby-dev
heroku addons:create heroku-redis:hobby-dev

4. Set the environment variables:

.. code-block:: bash

heroku config:set DJANGO_SETTINGS_MODULE=config.settings.production
heroku config:set DATABASE_URL=your-database-url
heroku config:set REDIS_URL=your-redis-url
heroku config:set CELERY_BROKER_URL=your-redis-url

Comment on lines +231 to +239
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Some rather important environment variables seem to be missing

The current set of environment variables might leave our application feeling a bit exposed. Consider adding:

heroku config:set DJANGO_SECRET_KEY=<your-secret-key>
heroku config:set DJANGO_ALLOWED_HOSTS=.herokuapp.com
heroku config:set DJANGO_DEBUG=False
heroku config:set CELERY_WORKER_CONCURRENCY=2  # adjust based on dyno size

5. Deploy the app:

.. code-block:: bash

git push heroku main

Comment on lines +240 to +245
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

The deployment section could use some verification steps

After pushing to Heroku, it would be nice to verify the deployment:

# Verify the deployment
heroku ps  # Check if dynos are running
heroku logs --tail  # Monitor the logs
heroku ps:scale worker=1  # Scale up Celery worker

6. Run the migrations and load the Redis index:

.. code-block:: bash

heroku run python manage.py migrate
heroku run python manage.py load_redis_index
Comment on lines +246 to +251
Copy link

@coderabbitai coderabbitai bot Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Running migrations directly in production? How adventurous!

Consider adding these safer practices:

  • Check migration status first: heroku run python manage.py showmigrations
  • Backup database before migrations: heroku pg:backups:capture
  • Verify Redis index after loading: heroku run python manage.py shell -c "from django.core.cache import cache; print(cache.keys('*'))"

Copy link
Member

Choose a reason for hiding this comment

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

Migrations should be run in production. Please update your Learning.

Copy link

Choose a reason for hiding this comment

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

@vigneshhari, thank you for the clarification. I've updated my understanding accordingly.

✏️ Learnings added 
Learnt from: vigneshhari
PR: ohcnetwork/care#2600
File: docs/local-setup/configuration.rst:246-251
Timestamp: 2024-11-16T17:24:49.641Z
Learning: In this project, running migrations directly in production is acceptable and intended practice.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.