From 69fc0d5a8cab479c347ea8d068b1073cafbf1082 Mon Sep 17 00:00:00 2001 From: Jvst Me Date: Tue, 28 Jan 2025 18:18:59 +0100 Subject: [PATCH] [chore]: Update/remove dstack-proxy TODOs --- docs/docs/reference/environment-variables.md | 2 +- src/dstack/_internal/proxy/lib/models.py | 2 +- src/dstack/_internal/proxy/lib/services/service_connection.py | 3 +-- .../_internal/server/services/proxy/routers/service_proxy.py | 3 --- .../_internal/server/services/proxy/services/service_proxy.py | 4 +--- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/docs/docs/reference/environment-variables.md b/docs/docs/reference/environment-variables.md index 319bbbe0d..569dfed04 100644 --- a/docs/docs/reference/environment-variables.md +++ b/docs/docs/reference/environment-variables.md @@ -103,7 +103,7 @@ For more details on the options below, refer to the [server deployment](../guide - `DSTACK_DATABASE_URL`{ #DSTACK_DATABASE_URL } – The database URL to use instead of default SQLite. Currently `dstack` supports Postgres. Example: `postgresql+asyncpg://myuser:mypassword@localhost:5432/mydatabase`. Defaults to `None`. - `DSTACK_SERVER_CLOUDWATCH_LOG_GROUP`{ #DSTACK_SERVER_CLOUDWATCH_LOG_GROUP } – The CloudWatch Logs group for workloads logs. If not set, the default file-based log storage is used. - `DSTACK_SERVER_CLOUDWATCH_LOG_REGION`{ #DSTACK_SERVER_CLOUDWATCH_LOG_REGION } – The CloudWatch Logs region. Defaults to `None`. -- `DSTACK_DEFAULT_SERVICE_CLIENT_MAX_BODY_SIZE`{ #DSTACK_DEFAULT_SERVICE_CLIENT_MAX_BODY_SIZE } – Request body size limit for services, in bytes. Defaults to 64 MiB. +- `DSTACK_DEFAULT_SERVICE_CLIENT_MAX_BODY_SIZE`{ #DSTACK_DEFAULT_SERVICE_CLIENT_MAX_BODY_SIZE } – Request body size limit for services running with a gateway, in bytes. Defaults to 64 MiB. - `DSTACK_FORBID_SERVICES_WITHOUT_GATEWAY`{ #DSTACK_FORBID_SERVICES_WITHOUT_GATEWAY } – Forbids registering new services without a gateway if set to any value. ??? info "Internal environment variables" diff --git a/src/dstack/_internal/proxy/lib/models.py b/src/dstack/_internal/proxy/lib/models.py index 9f107b070..5ec08f00a 100644 --- a/src/dstack/_internal/proxy/lib/models.py +++ b/src/dstack/_internal/proxy/lib/models.py @@ -31,7 +31,7 @@ class Service(ImmutableModel): domain: Optional[str] # only used on gateways https: Optional[bool] # only used on gateways auth: bool - client_max_body_size: int + client_max_body_size: int # only enforced on gateways replicas: tuple[Replica, ...] @property diff --git a/src/dstack/_internal/proxy/lib/services/service_connection.py b/src/dstack/_internal/proxy/lib/services/service_connection.py index df6ef14c0..a6d14ad13 100644 --- a/src/dstack/_internal/proxy/lib/services/service_connection.py +++ b/src/dstack/_internal/proxy/lib/services/service_connection.py @@ -87,7 +87,7 @@ async def client(self) -> ServiceClient: class ServiceConnectionPool: def __init__(self) -> None: - # TODO(#1595): remove connections to stopped replicas in-server + # TODO(#2238): remove connections to stopped replicas in-server self.connections: Dict[str, ServiceConnection] = {} async def get(self, replica_id: str) -> Optional[ServiceConnection]: @@ -139,7 +139,6 @@ async def get_service_replica_client( timeout=HTTP_TIMEOUT, ) # Nginx not available, forward directly to the tunnel - # TODO(#1595): consider trying different replicas, e.g. using HTTPMultiClient replica = random.choice(service.replicas) connection = await service_conn_pool.get(replica.id) if connection is None: diff --git a/src/dstack/_internal/server/services/proxy/routers/service_proxy.py b/src/dstack/_internal/server/services/proxy/routers/service_proxy.py index efc1bca7d..7660abf45 100644 --- a/src/dstack/_internal/server/services/proxy/routers/service_proxy.py +++ b/src/dstack/_internal/server/services/proxy/routers/service_proxy.py @@ -40,6 +40,3 @@ async def service_reverse_proxy( return await service_proxy.proxy( project_name, run_name, path, request, auth, repo, service_conn_pool ) - - -# TODO(#1595): support websockets diff --git a/src/dstack/_internal/server/services/proxy/services/service_proxy.py b/src/dstack/_internal/server/services/proxy/services/service_proxy.py index fc7836feb..3bbf9c136 100644 --- a/src/dstack/_internal/server/services/proxy/services/service_proxy.py +++ b/src/dstack/_internal/server/services/proxy/services/service_proxy.py @@ -26,8 +26,6 @@ async def proxy( repo: BaseProxyRepo, service_conn_pool: ServiceConnectionPool, ) -> fastapi.responses.Response: - # TODO(#1595): enforce client_max_body_size - if "Upgrade" in request.headers: raise ProxyError("Upgrading connections is not supported", status.HTTP_400_BAD_REQUEST) @@ -95,7 +93,7 @@ async def build_upstream_request( ).get_stream() client.cookies.clear() # the client is shared by all users, don't leak cookies - # TODO(#1595): add common proxy headers + # TODO(#2237): add common proxy headers return client.build_request( downstream_request.method, url, headers=downstream_request.headers, content=request_stream )