Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Use the existing and configured env indicator #476

Merged
merged 1 commit into from
Jan 20, 2022
Merged

Use the existing and configured env indicator #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions api/catalog/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = config("DJANGO_DEBUG_ENABLED", default=False, cast=bool)

PYTHON_ENV = config("PYTHON_ENV", default="production")
ENVIRONMENT = config("ENVIRONMENT", default="local")
Copy link
Member

Choose a reason for hiding this comment

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

Why did it change to "local"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the default that is used by the ingestion_server when it accesses this same variable:

openverse-api/ingestion_server/ingestion_server/distributed_reindex_scheduler.py

Line 73 in ad2f74a

environment = config("ENVIRONMENT", default="local")

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep "production" and change to "staging" or the corresponding string in the staging server. "local" doesn't seem very useful as everything is local to its own machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I agree that naming convention isn't too useful! 😅 I think dev or staging makes sense. A lot of the environment checks in the ingestion server are checking explicitly if in prod (environment == "prod") so it hasn't mattered much what not prod is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree local is ambiguous.

In my mind (and indeed I've seen this in practice) dev is local development, staging is the pre-production deployment, prod is production, and test is when tests are running regardless of environment. Also variations on those strings exist. Node (and most JavaScript projects like React and Vue) uses the full development and production specifically for example, so I like the consistency of using that for the less-opinionated Python environment.


ALLOWED_HOSTS = [
"api-dev.openverse.engineering",
Expand Down Expand Up @@ -318,5 +318,5 @@
integrations=[DjangoIntegration()],
traces_sample_rate=SENTRY_SAMPLE_RATE,
send_default_pii=False,
environment=PYTHON_ENV,
environment=ENVIRONMENT,
)
1 change: 0 additions & 1 deletion api/env.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
PYTHONUNBUFFERED="0"
PYTHON_ENV="development"

DJANGO_SETTINGS_MODULE="catalog.settings"
DJANGO_SECRET_KEY="ny#b__$$f6ry4wy8oxre97&-68u_0lk3gw(z=d40_dxey3zw0v1"
Expand Down