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.

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

[ON HOLD] Django settings cleanup, logging improvements and critical fixes #585

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ celerybeat.pid

# Environments
test*.env
*.env
!sample.env
.env
.*.env
.env.export
.venv*
env/
Expand Down Expand Up @@ -647,7 +647,8 @@ tools/*/data_dir/
backend/backend/settings/*
!backend/backend/settings/__init__.py
!backend/backend/settings/base.py
!backend/backend/settings/dev.py
!backend/backend/settings/platform.py
!backend/backend/settings/celery.py
!backend/backend/settings/test.py

# Local Dependencies for docker testing
Expand Down
2 changes: 1 addition & 1 deletion backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Contains the backend services for Unstract written with Django and DRF.
- Copy `sample.env` into `.env` and update the necessary variables. For eg:

```
DJANGO_SETTINGS_MODULE='backend.settings.dev'
DJANGO_SETTINGS_MODULE='backend.settings.platform'
DB_HOST='localhost'
DB_USER='unstract_dev'
DB_PASSWORD='unstract_pass'
Expand Down
3 changes: 2 additions & 1 deletion backend/account/authentication_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ def user_organizations(self, request: Request) -> Any:
try:
organizations = self.auth_service.user_organizations(request)
except Exception as ex:
#
Logger.error(f"Failed to get user orgs: {ex}")

self.user_logout(request)

response = Response(
Expand Down
9 changes: 3 additions & 6 deletions backend/account/authentication_plugin_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,11 @@ def _load_plugins() -> dict[str, dict[str, Any]]:
PluginConfig.AUTH_METADATA: module.metadata,
}
Logger.info(
"Loaded auth plugin: %s, is_active: %s",
module.metadata["name"],
module.metadata["is_active"],
"Loaded active authentication plugin: %s", module.metadata["name"]
)
else:
Logger.warning(
"Metadata is not active for %s authentication module.",
auth_module_name,
Logger.info(
"Skipping inactive authentication plugin: %s", auth_module_name
)
except ModuleNotFoundError as exception:
Logger.error(
Expand Down
3 changes: 0 additions & 3 deletions backend/backend/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
from .celery_service import app as celery_app

__all__ = ["celery_app"]
10 changes: 7 additions & 3 deletions backend/backend/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@

import os

from django.core.asgi import get_asgi_application
from dotenv import load_dotenv
from dotenv import find_dotenv, load_dotenv

load_dotenv(find_dotenv() or "")
Copy link
Contributor

Choose a reason for hiding this comment

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

@hari-kuriakose isn't this the default params / behaviour of the load_dotenv() ?

  1. Why do we call it this way?
  2. Isi it necessary to call load_dotenv() again?


load_dotenv()

os.environ.setdefault(
"DJANGO_SETTINGS_MODULE",
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.dev"),
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.platform"),
)

from django.core.asgi import get_asgi_application # noqa: E402
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this move to down? Is it for getting envs/settings before loading asgi?


application = get_asgi_application()
22 changes: 13 additions & 9 deletions backend/backend/celery_service.py → backend/backend/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,25 @@

import os

from celery import Celery
from django.conf import settings
from utils.constants import ExecutionLogConstants
from dotenv import find_dotenv, load_dotenv

from backend.celery_task import TaskRegistry

# Set the default Django settings module for the 'celery' program.
# NOTE:
# Do this before loading any environment variables.
os.environ.setdefault(
"DJANGO_SETTINGS_MODULE",
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.dev"),
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.celery"),
)

# Load environment variables.
load_dotenv(find_dotenv() or "")

from celery import Celery # noqa: E402
from django.conf import settings # noqa: E402
from utils.celery.constants import ExecutionLogConstants # noqa: E402
from utils.celery.task_registry import TaskRegistry # noqa: E402

# Create a Celery instance. Default time zone is UTC.
app = Celery("backend")
app = Celery("celery")

# To register the custom tasks.
TaskRegistry()
Expand All @@ -24,7 +29,6 @@
app.conf.broker_url = settings.CELERY_BROKER_URL
app.conf.result_backend = settings.CELERY_RESULT_BACKEND


# Load task modules from all registered Django app configs.
app.config_from_object("django.conf:settings", namespace="CELERY")

Expand Down
15 changes: 0 additions & 15 deletions backend/backend/flowerconfig.py

This file was deleted.

19 changes: 1 addition & 18 deletions backend/backend/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import os
from pathlib import Path
from typing import Optional
from urllib.parse import quote_plus

from dotenv import find_dotenv, load_dotenv
from utils.common_utils import CommonUtils
Expand Down Expand Up @@ -131,10 +130,6 @@ def get_required_setting(
LOG_HISTORY_CONSUMER_INTERVAL = int(
get_required_setting("LOG_HISTORY_CONSUMER_INTERVAL", "60")
)
LOGS_BATCH_LIMIT = int(get_required_setting("LOGS_BATCH_LIMIT", "30"))
CELERY_BROKER_URL = get_required_setting(
"CELERY_BROKER_URL", f"redis://{REDIS_HOST}:{REDIS_PORT}"
)

INDEXING_FLAG_TTL = int(get_required_setting("INDEXING_FLAG_TTL"))
NOTIFICATION_TIMEOUT = int(get_required_setting("NOTIFICATION_TIMEOUT", "5"))
Expand Down Expand Up @@ -367,22 +362,10 @@ def get_required_setting(
}


# CELERY_RESULT_BACKEND = f"redis://{REDIS_HOST}:{REDIS_PORT}/1"
# Postgres as result backend
CELERY_RESULT_BACKEND = (
f"db+postgresql://{DB_USER}:{quote_plus(DB_PASSWORD)}"
f"@{DB_HOST}:{DB_PORT}/{DB_NAME}"
)
CELERY_ACCEPT_CONTENT = ["json"]
CELERY_TASK_SERIALIZER = "json"
CELERY_RESULT_SERIALIZER = "json"
CELERY_TIMEZONE = "UTC"
CELERY_TASK_MAX_RETRIES = 3
CELERY_TASK_RETRY_BACKOFF = 60 # Time in seconds before retrying the task

# Feature Flag
FEATURE_FLAG_SERVICE_URL = {"evaluate": f"{FLIPT_BASE_URL}/api/v1/flags/evaluate/"}


# Password validation
# https://docs.djangoproject.com/en/4.2/ref/settings/#auth-password-validators

Expand Down
100 changes: 100 additions & 0 deletions backend/backend/settings/celery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import os
from typing import Any, Optional
from urllib.parse import quote_plus

from backend.constants import FeatureFlag

# Requires PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python env setting
# to be loaded prior.
from unstract.flags.feature_flag import check_feature_flag_status

missing_settings = []


def get_required_setting(
Copy link
Contributor

Choose a reason for hiding this comment

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

@hari-kuriakose we need to check missing_settings and raise error. Else using the get_required_setting wouldn't have any effect. Reference: https://github.com/Zipstack/unstract/blob/main/backend/backend/settings/base.py#L532-L536

setting_key: str, default: Optional[Any] = None
) -> Optional[str]:
"""Get the value of an environment variable specified by the given key. Add
missing keys to `missing_settings` so that exception can be raised at the
end.

Args:
key (str): The key of the environment variable
default (Optional[str], optional): Default value to return incase of
env not found. Defaults to None.

Returns:
Optional[str]: The value of the environment variable if found,
otherwise the default value.
"""
data = os.environ.get(setting_key, default)
if not data:
missing_settings.append(setting_key)
return data


DB_NAME = os.environ.get("DB_NAME", "unstract_db")
DB_USER = os.environ.get("DB_USER", "unstract_dev")
DB_HOST = os.environ.get("DB_HOST", "backend-db-1")
DB_PASSWORD = os.environ.get("DB_PASSWORD", "unstract_pass")
DB_PORT = os.environ.get("DB_PORT", 5432)
if check_feature_flag_status(FeatureFlag.MULTI_TENANCY_V2):
DB_ENGINE = "django.db.backends.postgresql"
else:
DB_ENGINE = "django_tenants.postgresql_backend"


REDIS_USER = os.environ.get("REDIS_USER", "default")
REDIS_PASSWORD = os.environ.get("REDIS_PASSWORD", "")
REDIS_HOST = os.environ.get("REDIS_HOST", "unstract-redis")
REDIS_PORT = os.environ.get("REDIS_PORT", "6379")
REDIS_DB = os.environ.get("REDIS_DB", "")

ENABLE_LOG_HISTORY = get_required_setting("ENABLE_LOG_HISTORY")
LOG_HISTORY_CONSUMER_INTERVAL = int(
get_required_setting("LOG_HISTORY_CONSUMER_INTERVAL", "60")
)
LOGS_BATCH_LIMIT = int(get_required_setting("LOGS_BATCH_LIMIT", "30"))

CORS_ALLOWED_ORIGINS = [
"http://localhost:3000",
"http://127.0.0.1:3000",
"http://frontend.unstract.localhost",
# Other allowed origins if needed
]

# Database
# https://docs.djangoproject.com/en/4.2/ref/settings/#databases
DATABASES = {
"default": {
"ENGINE": DB_ENGINE,
"NAME": f"{DB_NAME}",
"USER": f"{DB_USER}",
"HOST": f"{DB_HOST}",
"PASSWORD": f"{DB_PASSWORD}",
"PORT": f"{DB_PORT}",
"ATOMIC_REQUESTS": True,
}
}

# SocketIO connection manager
SOCKET_IO_MANAGER_URL = f"redis://{REDIS_HOST}:{REDIS_PORT}"

CELERY_BROKER_URL = get_required_setting(
"CELERY_BROKER_URL", f"redis://{REDIS_HOST}:{REDIS_PORT}"
)
# CELERY_RESULT_BACKEND = f"redis://{REDIS_HOST}:{REDIS_PORT}/1"
# Postgres as result backend
CELERY_RESULT_BACKEND = (
f"db+postgresql://{DB_USER}:{quote_plus(DB_PASSWORD)}"
f"@{DB_HOST}:{DB_PORT}/{DB_NAME}"
)
CELERY_ACCEPT_CONTENT = ["json"]
CELERY_TASK_SERIALIZER = "json"
CELERY_RESULT_SERIALIZER = "json"
CELERY_TIMEZONE = "UTC"
CELERY_TASK_MAX_RETRIES = 3
CELERY_TASK_RETRY_BACKOFF = 60 # Time in seconds before retrying the task


INSTALLED_APPS = ["django.contrib.contenttypes", "django_celery_beat"]
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from backend.settings.base import * # noqa: F401, F403

DEBUG = True
DEBUG = False

X_FRAME_OPTIONS = "http://localhost:3000"
X_FRAME_OPTIONS = "ALLOW-FROM http://localhost:3000"
Expand Down
13 changes: 7 additions & 6 deletions backend/backend/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,19 @@

import os

from django.conf import settings
from django.core.wsgi import get_wsgi_application
from dotenv import load_dotenv
from utils.log_events import start_server
from dotenv import find_dotenv, load_dotenv

load_dotenv()
load_dotenv(find_dotenv() or "")

os.environ.setdefault(
"DJANGO_SETTINGS_MODULE",
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.dev"),
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.platform"),
)

from django.conf import settings # noqa: E402
from django.core.wsgi import get_wsgi_application # noqa: E402
from utils.log_events import start_server # noqa: E402

django_app = get_wsgi_application()

application = start_server(django_app, f"{settings.PATH_PREFIX}/socket")
2 changes: 1 addition & 1 deletion backend/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def main() -> None:
load_dotenv()
os.environ.setdefault(
"DJANGO_SETTINGS_MODULE",
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.dev"),
os.environ.get("DJANGO_SETTINGS_MODULE", "backend.settings.platform"),
)
try:
from django.core.management import execute_from_command_line
Expand Down
Loading
Loading