Skip to content
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
5 changes: 2 additions & 3 deletions collectoss/api/gunicorn_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@
logs_directory = get_value('Logging', 'logs_directory')

# this syntax satisfies the type checker
is_docker = SystemEnv.get_bool("AUGUR_DOCKER_DEPLOY", False)
is_docker = SystemEnv.get_bool("COLLECTOSS_DOCKER_DEPLOY", False)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a change that was made in another PR already. can you rebase your PR on top of current main?

accesslog = f"{logs_directory}/gunicorn.log"
errorlog = f"{logs_directory}/gunicorn.log"

# If deploying via docker, include gunicorn error logs in the docker log stream by sending it to stdout
# If deploying via Docker, include Gunicorn error logs in the Docker log stream.
if is_docker:
errorlog = '-'

Expand All @@ -69,4 +69,3 @@
def worker_exit(server, worker):
print("Stopping gunicorn worker process")
dispose_database_engine()

46 changes: 46 additions & 0 deletions collectoss/application/cli/_gunicorn.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you explain why this was put in its own file/made into a function?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The Gunicorn command now requires deployment-specific logic (Docker vs non-Docker). Extracting it into build_gunicorn_command() centralizes command construction, makes the behavior testable, avoids duplicating Docker logging logic elsewhere, and keeps start() focused on process startup rather than command assembly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the non-docker usecase is largely deprecated in collectoss, and just hasn't really been removed yet

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# SPDX-License-Identifier: MIT
"""
Helpers for starting Gunicorn from CollectOSS CLI commands.
"""
from __future__ import annotations

import os
from typing import Optional

from collectoss.application.environment import SystemEnv


def is_docker_deploy() -> bool:
return SystemEnv.get_bool("COLLECTOSS_DOCKER_DEPLOY", False)


def build_gunicorn_command(
gunicorn_config: str,
host: str,
port: str,
app: str = "collectoss.api.server:app",
log_file: Optional[str] = None,
docker_deploy: Optional[bool] = None,
) -> list[str]:
"""
Build a Gunicorn command while preserving Docker stderr logging.

In Docker, Gunicorn's config sends error logs to "-" so failures are visible
through container logs. Passing --log-file would override that setting.
"""
command = [
"gunicorn",
"-c",
gunicorn_config,
"-b",
f"{host}:{port}",
app,
]

if docker_deploy is None:
docker_deploy = is_docker_deploy()

if log_file and not docker_deploy:
command.extend(["--log-file", os.fspath(log_file)])

return command
7 changes: 3 additions & 4 deletions collectoss/application/cli/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from collectoss.application.logs import SystemLogger
from collectoss.application.cli import test_connection, test_db_connection, with_database, DatabaseContext
from collectoss.application.cli._cli_util import _broadcast_signal_to_processes, raise_open_file_limit, clear_redis_caches, clear_rabbitmq_messages
from collectoss.application.cli._gunicorn import build_gunicorn_command
from collectoss.application.db.lib import get_value
from collectoss.application.environment import SystemEnv

Expand Down Expand Up @@ -61,8 +62,8 @@ def start(ctx, development, port):
if not port:
port = get_value("Server", "port")

gunicorn_command = f"gunicorn -c {gunicorn_location} -b {host}:{port} collectoss.api.server:app --log-file gunicorn.log"
server = subprocess.Popen(gunicorn_command.split(" "))
gunicorn_command = build_gunicorn_command(gunicorn_location, host, str(port), log_file="gunicorn.log")
server = subprocess.Popen(gunicorn_command)

time.sleep(3)
logger.info('Gunicorn webserver started...')
Expand Down Expand Up @@ -154,5 +155,3 @@ def is_api_process(process):
return True

return False


5 changes: 3 additions & 2 deletions collectoss/application/cli/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from collectoss.application.service_manager import SystemServiceManager, cleanup_collection_status_and_rabbit, clean_collection_status
from collectoss.application.db.lib import get_value
from collectoss.application.cli import test_connection, test_db_connection, with_database, DatabaseContext
from collectoss.application.cli._gunicorn import build_gunicorn_command
import sqlalchemy as s

from keyman.KeyClient import KeyClient, KeyPublisher
Expand Down Expand Up @@ -104,8 +105,8 @@ def start(ctx, disable_collection, development, pidfile, port):
log_dir = get_value("Logging", "logs_directory") or "."
gunicorn_log_file = os.path.join(log_dir, "gunicorn.log")

gunicorn_command = f"gunicorn -c {gunicorn_location} -b {host}:{port} collectoss.api.server:app --log-file {gunicorn_log_file}"
server = subprocess.Popen(gunicorn_command.split(" "))
gunicorn_command = build_gunicorn_command(gunicorn_location, host, str(port), log_file=gunicorn_log_file)
server = subprocess.Popen(gunicorn_command)
manager.server = server

logger.info("awaiting Gunicorn start")
Expand Down
45 changes: 45 additions & 0 deletions tests/test_application/test_cli/test_gunicorn_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# SPDX-License-Identifier: MIT
"""Tests for Gunicorn CLI command construction."""

from collectoss.application.cli._gunicorn import build_gunicorn_command


def test_gunicorn_command_uses_log_file_outside_docker():
command = build_gunicorn_command(
"/collectoss/collectoss/api/gunicorn_conf.py",
"0.0.0.0",
"5000",
log_file="/collectoss/logs/gunicorn.log",
docker_deploy=False,
)

assert command == [
"gunicorn",
"-c",
"/collectoss/collectoss/api/gunicorn_conf.py",
"-b",
"0.0.0.0:5000",
"collectoss.api.server:app",
"--log-file",
"/collectoss/logs/gunicorn.log",
]


def test_gunicorn_command_does_not_override_docker_errorlog():
command = build_gunicorn_command(
"/collectoss/collectoss/api/gunicorn_conf.py",
"0.0.0.0",
"5000",
log_file="/collectoss/logs/gunicorn.log",
docker_deploy=True,
)

assert "--log-file" not in command
assert command == [
"gunicorn",
"-c",
"/collectoss/collectoss/api/gunicorn_conf.py",
"-b",
"0.0.0.0:5000",
"collectoss.api.server:app",
]