From debe1b9e80595ba0c369a2ea9450bf3116b81289 Mon Sep 17 00:00:00 2001 From: Matteo Ferrando Date: Sun, 15 Sep 2024 12:25:07 -0500 Subject: [PATCH 1/4] refactor: make IsolateLogger more explicit --- src/isolate/logger.py | 41 ++++++++++++++++++++++++------------ src/isolate/server/server.py | 5 +++-- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/isolate/logger.py b/src/isolate/logger.py index fdd7a1f..473fe07 100644 --- a/src/isolate/logger.py +++ b/src/isolate/logger.py @@ -1,24 +1,17 @@ import json import os +from isolate.logs import LogLevel, LogSource + # NOTE: we probably should've created a proper `logging.getLogger` here, # but it handling `source` would be not trivial, so we are better off # just keeping it simple for now. class IsolateLogger: - def __init__(self): - self.log_labels = {} - raw = os.getenv("ISOLATE_LOG_LABELS") - if raw: - labels = json.loads(raw) - for key, value in labels.items(): - if value.startswith("$"): - expanded = os.getenv(value[1:]) - else: - expanded = value - self.log_labels[key] = expanded - - def log(self, level, message, source): + def __init__(self, log_labels: dict[str, str]): + self.log_labels = log_labels + + def log(self, level: LogLevel, message: str, source: LogSource): record = { "isolate_source": source.name, "level": level.name, @@ -27,5 +20,25 @@ def log(self, level, message, source): } print(json.dumps(record)) + @classmethod + def with_env_expanded(cls, labels: dict[str, str]): + for key, value in labels.items(): + if value.startswith("$"): + expanded = os.getenv(value[1:]) + else: + expanded = value + if expanded is not None: + labels[key] = expanded + + return cls(labels) + + +_labels = {} +try: + raw = os.getenv("ISOLATE_LOG_LABELS") + if raw: + _labels: dict[str, str] = json.loads(raw) +except BaseException: + print("Failed to parse ISOLATE_LOG_LABELS") -logger = IsolateLogger() +ENV_LOGGER = IsolateLogger.with_env_expanded(labels=_labels) diff --git a/src/isolate/server/server.py b/src/isolate/server/server.py index 67fb2a4..7bdb441 100644 --- a/src/isolate/server/server.py +++ b/src/isolate/server/server.py @@ -29,7 +29,7 @@ from isolate.backends.virtualenv import VirtualPythonEnvironment from isolate.connections.grpc import AgentError, LocalPythonGRPC from isolate.connections.grpc.configuration import get_default_options -from isolate.logger import logger +from isolate.logger import ENV_LOGGER, IsolateLogger from isolate.logs import Log, LogLevel, LogSource from isolate.server import definitions, health from isolate.server.health_server import HealthServicer @@ -453,9 +453,10 @@ def _proxy_to_queue( @dataclass class LogHandler: messages: Queue + logger: IsolateLogger = ENV_LOGGER def handle(self, log: Log) -> None: - logger.log(log.level, log.message, source=log.source) + self.logger.log(log.level, log.message, source=log.source) self._add_log_to_queue(log) def _add_log_to_queue(self, log: Log) -> None: From 089eebd1862a9baed92c410b4c29216f2beea66b Mon Sep 17 00:00:00 2001 From: Matteo Ferrando Date: Mon, 16 Sep 2024 13:10:28 -0400 Subject: [PATCH 2/4] Update src/isolate/logger.py Co-authored-by: Ruslan Kuprieiev --- src/isolate/logger.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/isolate/logger.py b/src/isolate/logger.py index 473fe07..9393972 100644 --- a/src/isolate/logger.py +++ b/src/isolate/logger.py @@ -34,11 +34,11 @@ def with_env_expanded(cls, labels: dict[str, str]): _labels = {} -try: - raw = os.getenv("ISOLATE_LOG_LABELS") - if raw: +raw = os.getenv("ISOLATE_LOG_LABELS") +if raw: + try: _labels: dict[str, str] = json.loads(raw) -except BaseException: - print("Failed to parse ISOLATE_LOG_LABELS") + except json.JSONDecodeError: + print("Failed to parse ISOLATE_LOG_LABELS") ENV_LOGGER = IsolateLogger.with_env_expanded(labels=_labels) From faae97b2cb227d569d47f114adacb4ce3a09e769 Mon Sep 17 00:00:00 2001 From: Matteo Ferrando Date: Mon, 16 Sep 2024 17:27:05 -0700 Subject: [PATCH 3/4] typing --- src/isolate/logger.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/isolate/logger.py b/src/isolate/logger.py index 9393972..8b96367 100644 --- a/src/isolate/logger.py +++ b/src/isolate/logger.py @@ -1,5 +1,6 @@ import json import os +from typing import Dict from isolate.logs import LogLevel, LogSource @@ -8,10 +9,10 @@ # but it handling `source` would be not trivial, so we are better off # just keeping it simple for now. class IsolateLogger: - def __init__(self, log_labels: dict[str, str]): + def __init__(self, log_labels: Dict[str, str]): self.log_labels = log_labels - def log(self, level: LogLevel, message: str, source: LogSource): + def log(self, level: LogLevel, message: str, source: LogSource) -> None: record = { "isolate_source": source.name, "level": level.name, @@ -21,7 +22,7 @@ def log(self, level: LogLevel, message: str, source: LogSource): print(json.dumps(record)) @classmethod - def with_env_expanded(cls, labels: dict[str, str]): + def with_env_expanded(cls, labels: Dict[str, str]) -> "IsolateLogger": for key, value in labels.items(): if value.startswith("$"): expanded = os.getenv(value[1:]) @@ -33,11 +34,11 @@ def with_env_expanded(cls, labels: dict[str, str]): return cls(labels) -_labels = {} +_labels: Dict[str, str] = {} raw = os.getenv("ISOLATE_LOG_LABELS") if raw: try: - _labels: dict[str, str] = json.loads(raw) + _labels = json.loads(raw) except json.JSONDecodeError: print("Failed to parse ISOLATE_LOG_LABELS") From 9c1389a04acf613bec5543655b9f1f1b57ae3b11 Mon Sep 17 00:00:00 2001 From: Matteo Ferrando Date: Mon, 16 Sep 2024 17:46:21 -0700 Subject: [PATCH 4/4] tests --- src/isolate/logger.py | 20 ++++++++++++-------- tests/test_logger.py | 35 +++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/isolate/logger.py b/src/isolate/logger.py index 8b96367..3065b9e 100644 --- a/src/isolate/logger.py +++ b/src/isolate/logger.py @@ -33,13 +33,17 @@ def with_env_expanded(cls, labels: Dict[str, str]) -> "IsolateLogger": return cls(labels) + @classmethod + def from_env(cls) -> "IsolateLogger": + _labels: Dict[str, str] = {} + raw = os.getenv("ISOLATE_LOG_LABELS") + if raw: + try: + _labels = json.loads(raw) + except json.JSONDecodeError: + print("Failed to parse ISOLATE_LOG_LABELS") + + return cls.with_env_expanded(labels=_labels) -_labels: Dict[str, str] = {} -raw = os.getenv("ISOLATE_LOG_LABELS") -if raw: - try: - _labels = json.loads(raw) - except json.JSONDecodeError: - print("Failed to parse ISOLATE_LOG_LABELS") -ENV_LOGGER = IsolateLogger.with_env_expanded(labels=_labels) +ENV_LOGGER = IsolateLogger.from_env() diff --git a/tests/test_logger.py b/tests/test_logger.py index b8d624f..794ca39 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -1,22 +1,45 @@ import json +import pytest from isolate.logger import IsolateLogger -def test_logger(monkeypatch): - labels = { +@pytest.fixture +def log_labels(): + return { "foo": "$MYENVVAR1", "bar": "$MYENVVAR2", "baz": "baz", - "qux": "qux", + "qux": "$NOTTHERE", } + + +def test_logger_with_env_expanded(log_labels, monkeypatch): monkeypatch.setenv("MYENVVAR1", "myenvvar1") monkeypatch.setenv("MYENVVAR2", "myenvvar2") - monkeypatch.setenv("ISOLATE_LOG_LABELS", json.dumps(labels)) - logger = IsolateLogger() + logger = IsolateLogger.with_env_expanded(log_labels) assert logger.log_labels == { "foo": "myenvvar1", "bar": "myenvvar2", "baz": "baz", - "qux": "qux", + "qux": "$NOTTHERE", } + + +def test_logger_from_env(log_labels, monkeypatch): + monkeypatch.setenv("MYENVVAR1", "myenvvar1") + monkeypatch.setenv("MYENVVAR2", "myenvvar2") + monkeypatch.setenv("ISOLATE_LOG_LABELS", json.dumps(log_labels)) + logger = IsolateLogger.from_env() + assert logger.log_labels == { + "foo": "myenvvar1", + "bar": "myenvvar2", + "baz": "baz", + "qux": "$NOTTHERE", + } + + +def test_logger_direct(log_labels): + logger = IsolateLogger(log_labels=log_labels) + # should not do env expansion + assert logger.log_labels == log_labels