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

Jump to bottom

Use POSTGRES_* and REDIS_URL as fallback #4811

Closed
wants to merge 2 commits into from
Closed

Use POSTGRES_* and REDIS_URL as fallback #4811

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77a974d
Try POSTGRES_ vars when DATABASE_URL is improperly configured
foarsitter Jan 27, 2024
19cf1cd
Fallback to REDIS_URL when CELERY_BROKER_URL is missing
foarsitter Jan 27, 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
38 changes: 27 additions & 11 deletions {{cookiecutter.project_slug}}/config/settings/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,28 @@
# DATABASES
# ------------------------------------------------------------------------------
# https://docs.djangoproject.com/en/dev/ref/settings/#databases
{% if cookiecutter.use_docker == "y" -%}
DATABASES = {"default": env.db("DATABASE_URL")}
{%- else %}
DATABASES = {
"default": env.db(
"DATABASE_URL",
default="postgres://{% if cookiecutter.windows == 'y' %}localhost{% endif %}/{{cookiecutter.project_slug}}",
),
}
{%- endif %}
try:
{%- if cookiecutter.use_docker == "y" %}
DATABASES = {"default": env.db("DATABASE_URL")}
{%- else %}
DATABASES = {
"default": env.db(
"DATABASE_URL",
default="postgres://{% if cookiecutter.windows == 'y' %}localhost{% endif %}/{{cookiecutter.project_slug}}",
),
}
{%- endif %}
except environ.ImproperlyConfigured:
DATABASES = {
"default": {
"ENGINE": "django.db.backends.postgresql",
"NAME": env.str("POSTGRES_DB"),
"USER": env.str("POSTGRES_USER"),
"PASSWORD": env.str("POSTGRES_PASSWORD"),
"HOST": env.str("POSTGRES_HOST"),
"PORT": env.str("POSTGRES_PORT"),
}
}
Comment on lines +58 to +68
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of this except 🤔 I can't quite really put a finger on why, though (sorry, that's unhelpful)

Copy link
Member

@browniebroke browniebroke Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's because in case of problems, it becomes harder to figure out where the DB credentials are coming from. Do I have a DATABASE_URL set? Do I have anything missing from the required POSTGRES_... env?

Sure, it's only one more place to check, but it can be really confusing when your app won't conect to the DB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Relaying on exceptions here is a lazy way of doing things I suppose. When a faulty url is given it would try to use the postgres vars, so I agree on your reluctance and it should be improved.

DATABASES["default"]["ATOMIC_REQUESTS"] = True
# https://docs.djangoproject.com/en/stable/ref/settings/#std:setting-DEFAULT_AUTO_FIELD
DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"
Expand Down Expand Up @@ -285,7 +297,11 @@
# https://docs.celeryq.dev/en/stable/userguide/configuration.html#std:setting-timezone
CELERY_TIMEZONE = TIME_ZONE
# https://docs.celeryq.dev/en/stable/userguide/configuration.html#std:setting-broker_url
CELERY_BROKER_URL = env("CELERY_BROKER_URL")
try:
CELERY_BROKER_URL = env("CELERY_BROKER_URL")
except environ.ImproperlyConfigured:
CELERY_BROKER_URL = env("REDIS_URL")
Comment on lines +300 to +303
Copy link
Member

Choose a reason for hiding this comment

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

When I look at this, I wonder why we need CELERY_BROKER_URL in the first place 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For a project, in the time that Heroku had free redis instances, I used a three instances: a cache backend, celery broker and result backend. But yeah, we can get rid of it since it doesn't add any value.


# https://docs.celeryq.dev/en/stable/userguide/configuration.html#std:setting-result_backend
CELERY_RESULT_BACKEND = CELERY_BROKER_URL
# https://docs.celeryq.dev/en/stable/userguide/configuration.html#result-extended
Expand Down