Skip to content

Commit

Permalink
[ENH]: CLI log path parameter support (#1631)
Browse files Browse the repository at this point in the history
Refs: #1624, #1591

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
- Added `--log--path` support to the CLI. A naive implementation that
works with Chroma's `log_config.yml`

## Test plan
*How are these changes tested?*

- [x] Tests pass locally with `pytest` for python

## Documentation Changes

TBD
  • Loading branch information
tazarov authored Jan 15, 2024
1 parent 4407b26 commit 3360399
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
28 changes: 18 additions & 10 deletions chromadb/cli/cli.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import logging
from typing import Optional

import yaml
from typing_extensions import Annotated
import typer
import uvicorn
import os
import webbrowser

from chromadb.cli.utils import set_log_file_path

app = typer.Typer()

_logo = """
Expand All @@ -25,14 +30,17 @@

@app.command() # type: ignore
def run(
path: str = typer.Option(
"./chroma_data", help="The path to the file or directory."
),
host: Annotated[
Optional[str], typer.Option(help="The host to listen to. Default: localhost")
] = "localhost",
port: int = typer.Option(8000, help="The port to run the server on."),
test: bool = typer.Option(False, help="Test mode.", show_envvar=False, hidden=True),
path: str = typer.Option(
"./chroma_data", help="The path to the file or directory."
),
host: Annotated[
Optional[str], typer.Option(help="The host to listen to. Default: localhost")
] = "localhost",
log_path: Annotated[
Optional[str], typer.Option(help="The path to the log file.")
] = "chroma.log",
port: int = typer.Option(8000, help="The port to run the server on."),
test: bool = typer.Option(False, help="Test mode.", show_envvar=False, hidden=True),
) -> None:
"""Run a chroma server"""

Expand Down Expand Up @@ -60,13 +68,13 @@ def run(

# this is the path of the CLI, we want to move up one directory
chromadb_path = os.path.dirname(chromadb_path)

log_config = set_log_file_path(f"{chromadb_path}/log_config.yml", f"{log_path}")
config = {
"app": "chromadb.app:app",
"host": host,
"port": port,
"workers": 1,
"log_config": f"{chromadb_path}/log_config.yml",
"log_config": log_config, # Pass the modified log_config dictionary
"timeout_keep_alive": 30,
}

Expand Down
15 changes: 15 additions & 0 deletions chromadb/cli/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from typing import Any, Dict

import yaml


def set_log_file_path(log_config_path: str, new_filename: str = "chroma.log") -> Dict[str, Any]:
"""This works with the standard log_config.yml file.
It will not work with custom log configs that may use different handlers"""
with open(f"{log_config_path}", 'r') as file:
log_config = yaml.safe_load(file)
for handler in log_config['handlers'].values():
if handler.get('class') == 'logging.handlers.RotatingFileHandler':
handler['filename'] = new_filename

return log_config
6 changes: 6 additions & 0 deletions chromadb/test/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typer.testing import CliRunner

from chromadb.cli.cli import app
from chromadb.cli.utils import set_log_file_path

runner = CliRunner()

Expand All @@ -19,3 +20,8 @@ def test_app() -> None:
)
assert "chroma_test_data" in result.stdout
assert "8001" in result.stdout


def test_utils_set_log_file_path() -> None:
log_config = set_log_file_path("chromadb/log_config.yml", "test.log")
assert log_config["handlers"]["file"]["filename"] == "test.log"

0 comments on commit 3360399

Please sign in to comment.