Skip to content

Commit

Permalink
Merge pull request #9248 from OpenMined/feat/improve-outputs
Browse files Browse the repository at this point in the history
Improve Logging outputs
  • Loading branch information
koenvanderveen authored Sep 3, 2024
2 parents c2e2b41 + 309d81d commit 2811e38
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 18 deletions.
21 changes: 21 additions & 0 deletions packages/syft/src/syft/deployment_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Can also be specified by the environment variable
# ORCHESTRA_DEPLOYMENT_TYPE
# stdlib
from enum import Enum

# relative
from .serde.serializable import serializable
from .types.syft_object import SYFT_OBJECT_VERSION_1


@serializable()
class DeploymentType(str, Enum):
__canonical_name__ = "DeploymentType"
__version__ = SYFT_OBJECT_VERSION_1

PYTHON = "python"
REMOTE = "remote"

def __str__(self) -> str:
# Use values when transforming ServerType to str
return self.value
14 changes: 6 additions & 8 deletions packages/syft/src/syft/orchestra.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

# stdlib
from collections.abc import Callable
from enum import Enum
import getpass
import inspect
import json
Expand All @@ -23,6 +22,7 @@
from .abstract_server import ServerType
from .client.client import login as sy_login
from .client.client import login_as_guest as sy_login_as_guest
from .deployment_type import DeploymentType
from .protocol.data_protocol import stage_protocol_changes
from .server.datasite import Datasite
from .server.enclave import Enclave
Expand Down Expand Up @@ -65,13 +65,6 @@ def get_deployment_type(deployment_type: str | None) -> DeploymentType | None:
return None


# Can also be specified by the environment variable
# ORCHESTRA_DEPLOYMENT_TYPE
class DeploymentType(Enum):
PYTHON = "python"
REMOTE = "remote"


class ServerHandle:
def __init__(
self,
Expand Down Expand Up @@ -186,6 +179,7 @@ def deploy_to_python(
queue_port: int | None = None,
association_request_auto_approval: bool = False,
background_tasks: bool = False,
log_level: str | int | None = None,
debug: bool = False,
migrate: bool = False,
) -> ServerHandle:
Expand Down Expand Up @@ -214,9 +208,11 @@ def deploy_to_python(
"n_consumers": n_consumers,
"create_producer": create_producer,
"association_request_auto_approval": association_request_auto_approval,
"log_level": log_level,
"background_tasks": background_tasks,
"debug": debug,
"migrate": migrate,
"deployment_type": deployment_type_enum,
}

if port:
Expand Down Expand Up @@ -316,6 +312,7 @@ def launch(
local_db: bool = False,
dev_mode: bool = False,
reset: bool = False,
log_level: str | int | None = None,
tail: bool = False,
host: str | None = "0.0.0.0", # nosec
enable_warnings: bool = False,
Expand Down Expand Up @@ -367,6 +364,7 @@ def launch(
local_db=local_db,
server_side_type=server_side_type_enum,
enable_warnings=enable_warnings,
log_level=log_level,
n_consumers=n_consumers,
thread_workers=thread_workers,
create_producer=create_producer,
Expand Down
2 changes: 1 addition & 1 deletion packages/syft/src/syft/protocol/protocol_version.json
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@
"WorkerSettings": {
"1": {
"version": 1,
"hash": "b8ddbe75a95b0312a64b5f25e0fc415aeac5c391975d02b515db8c848482f737",
"hash": "dca33003904a71688e5b07db65f8833eb4de8135aade7154076b8eafbb94d26b",
"action": "add"
}
},
Expand Down
44 changes: 44 additions & 0 deletions packages/syft/src/syft/server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from ..client.api import SyftAPIData
from ..client.api import debox_signed_syftapicall_response
from ..client.client import SyftClient
from ..deployment_type import DeploymentType
from ..protocol.data_protocol import PROTOCOL_TYPE
from ..protocol.data_protocol import get_data_protocol
from ..service.action.action_object import Action
Expand Down Expand Up @@ -315,6 +316,7 @@ def __init__(
processes: int = 0,
is_subprocess: bool = False,
server_type: str | ServerType = ServerType.DATASITE,
deployment_type: str | DeploymentType = "remote",
local_db: bool = False,
reset: bool = False,
blob_storage_config: BlobStorageConfig | None = None,
Expand All @@ -328,6 +330,7 @@ def __init__(
dev_mode: bool = False,
migrate: bool = False,
in_memory_workers: bool = True,
log_level: int | None = None,
smtp_username: str | None = None,
smtp_password: str | None = None,
email_sender: str | None = None,
Expand All @@ -353,8 +356,16 @@ def __init__(

if isinstance(server_type, str):
server_type = ServerType(server_type)

self.server_type = server_type

if isinstance(deployment_type, str):
deployment_type = DeploymentType(deployment_type)
self.deployment_type = deployment_type

# do this after we set the deployment type
self.set_log_level(log_level)

if isinstance(server_side_type, str):
server_side_type = ServerSideType(server_side_type)
self.server_side_type = server_side_type
Expand Down Expand Up @@ -443,6 +454,37 @@ def __init__(

ServerRegistry.set_server_for(self.id, self)

def set_log_level(self, log_level: int | str | None) -> None:
def determine_log_level(
log_level: str | int | None, default: int
) -> int | None:
if log_level is None:
return default
if isinstance(log_level, str):
level = logging.getLevelName(log_level.upper())
if isinstance(level, str) and level.startswith("Level "):
level = logging.INFO # defaults to info otherwise
return level # type: ignore
return log_level

default = logging.CRITICAL
if self.deployment_type == DeploymentType.PYTHON:
default = logging.CRITICAL
elif self.dev_mode: # if real deployment and dev mode
default = logging.INFO

self.log_level = determine_log_level(log_level, default)

logging.getLogger().setLevel(self.log_level)

if log_level == logging.DEBUG:
# only do this if specifically set, very noisy
logging.getLogger("uvicorn").setLevel(logging.DEBUG)
logging.getLogger("uvicorn.access").setLevel(logging.DEBUG)
else:
logging.getLogger("uvicorn").setLevel(logging.CRITICAL)
logging.getLogger("uvicorn.access").setLevel(logging.CRITICAL)

@property
def runs_in_docker(self) -> bool:
path = "/proc/self/cgroup"
Expand Down Expand Up @@ -674,6 +716,7 @@ def named(
local_db: bool = False,
server_type: str | ServerType = ServerType.DATASITE,
server_side_type: str | ServerSideType = ServerSideType.HIGH_SIDE,
deployment_type: str | DeploymentType = "remote",
enable_warnings: bool = False,
n_consumers: int = 0,
thread_workers: bool = False,
Expand Down Expand Up @@ -701,6 +744,7 @@ def named(
local_db=local_db,
server_type=server_type,
server_side_type=server_side_type,
deployment_type=deployment_type,
enable_warnings=enable_warnings,
blob_storage_config=blob_storage_config,
queue_port=queue_port,
Expand Down
14 changes: 7 additions & 7 deletions packages/syft/src/syft/server/uvicorn.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# stdlib
from collections.abc import Callable
from contextlib import asynccontextmanager
import logging
import multiprocessing
import multiprocessing.synchronize
import os
Expand All @@ -25,6 +24,7 @@
# relative
from ..abstract_server import ServerSideType
from ..client.client import API_PATH
from ..deployment_type import DeploymentType
from ..util.autoreload import enable_autoreload
from ..util.constants import DEFAULT_TIMEOUT
from ..util.telemetry import TRACING_ENABLED
Expand All @@ -50,6 +50,7 @@ class AppSettings(BaseSettings):
name: str
server_type: ServerType = ServerType.DATASITE
server_side_type: ServerSideType = ServerSideType.HIGH_SIDE
deployment_type: DeploymentType = DeploymentType.REMOTE
processes: int = 1
reset: bool = False
dev_mode: bool = False
Expand Down Expand Up @@ -147,6 +148,7 @@ def run_uvicorn(
starting_uvicorn_event: multiprocessing.synchronize.Event,
**kwargs: Any,
) -> None:
log_level = kwargs.get("log_level")
dev_mode = kwargs.get("dev_mode")
should_reset = dev_mode and kwargs.get("reset")

Expand All @@ -166,12 +168,6 @@ def run_uvicorn(
except Exception: # nosec
print(f"Failed to kill python process on port: {port}")

log_level = "critical"
if dev_mode:
log_level = "info"
logging.getLogger("uvicorn").setLevel(logging.CRITICAL)
logging.getLogger("uvicorn.access").setLevel(logging.CRITICAL)

if kwargs.get("debug"):
attach_debugger()

Expand Down Expand Up @@ -208,6 +204,7 @@ def serve_server(
name: str,
server_type: ServerType = ServerType.DATASITE,
server_side_type: ServerSideType = ServerSideType.HIGH_SIDE,
deployment_type: DeploymentType = DeploymentType.REMOTE,
host: str = "0.0.0.0", # nosec
port: int = 8080,
processes: int = 1,
Expand All @@ -216,6 +213,7 @@ def serve_server(
tail: bool = False,
enable_warnings: bool = False,
in_memory_workers: bool = True,
log_level: str | int | None = None,
queue_port: int | None = None,
create_producer: bool = False,
n_consumers: int = 0,
Expand All @@ -242,13 +240,15 @@ def serve_server(
"server_side_type": server_side_type,
"enable_warnings": enable_warnings,
"in_memory_workers": in_memory_workers,
"log_level": log_level,
"queue_port": queue_port,
"create_producer": create_producer,
"n_consumers": n_consumers,
"association_request_auto_approval": association_request_auto_approval,
"background_tasks": background_tasks,
"debug": debug,
"starting_uvicorn_event": starting_uvicorn_event,
"deployment_type": deployment_type,
},
)

Expand Down
5 changes: 5 additions & 0 deletions packages/syft/src/syft/server/worker_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from ..abstract_server import AbstractServer
from ..abstract_server import ServerSideType
from ..abstract_server import ServerType
from ..deployment_type import DeploymentType
from ..serde.serializable import serializable
from ..server.credentials import SyftSigningKey
from ..service.queue.base_queue import QueueConfig
Expand All @@ -27,11 +28,13 @@ class WorkerSettings(SyftObject):
name: str
server_type: ServerType
server_side_type: ServerSideType
deployment_type: DeploymentType = DeploymentType.REMOTE
signing_key: SyftSigningKey
document_store_config: StoreConfig
action_store_config: StoreConfig
blob_store_config: BlobStorageConfig | None = None
queue_config: QueueConfig | None = None
log_level: int | None = None

@classmethod
def from_server(cls, server: AbstractServer) -> Self:
Expand All @@ -49,4 +52,6 @@ def from_server(cls, server: AbstractServer) -> Self:
server_side_type=server_side_type,
blob_store_config=server.blob_store_config,
queue_config=server.queue_config,
log_level=server.log_level,
deployment_type=server.deployment_type,
)
2 changes: 2 additions & 0 deletions packages/syft/src/syft/service/queue/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ def handle_message_multiprocessing(
queue_config=queue_config,
is_subprocess=True,
migrate=False,
deployment_type=worker_settings.deployment_type,
)

# Set monitor thread for this job.
Expand Down Expand Up @@ -264,6 +265,7 @@ def handle_message(message: bytes, syft_worker_id: UID) -> None:
action_store_config=worker_settings.action_store_config,
blob_storage_config=worker_settings.blob_store_config,
server_side_type=worker_settings.server_side_type,
deployment_type=worker_settings.deployment_type,
queue_config=queue_config,
is_subprocess=True,
migrate=False,
Expand Down
2 changes: 0 additions & 2 deletions packages/syft/src/syft/service/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import TYPE_CHECKING

# third party
from IPython.display import display
from typing_extensions import Self

# relative
Expand Down Expand Up @@ -51,7 +50,6 @@ def __getattr__(self, name: str) -> Any:
"__version__",
] or name.startswith("_repr"):
return super().__getattr__(name)
display(self)
raise AttributeError(
f"You have tried accessing `{name}` on a {type(self).__name__} with message: {self.message}"
)
Expand Down

0 comments on commit 2811e38

Please sign in to comment.