Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Version mismatch checks #3989

Merged
merged 6 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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: 3 additions & 2 deletions src/phoenix/experiments/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,16 @@
)
from phoenix.experiments.utils import get_dataset_experiments_url, get_experiment_url, get_func_name
from phoenix.trace.attributes import flatten
from phoenix.utilities.client import VersionedAsyncClient, VersionedClient
from phoenix.utilities.json import jsonify


def _phoenix_clients() -> Tuple[httpx.Client, httpx.AsyncClient]:
headers = get_env_client_headers()
return httpx.Client(
return VersionedClient(
base_url=get_base_url(),
headers=headers,
), httpx.AsyncClient(
), VersionedAsyncClient(
base_url=get_base_url(),
headers=headers,
)
Expand Down
4 changes: 4 additions & 0 deletions src/phoenix/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
from phoenix.server.grpc_server import GrpcServer
from phoenix.server.telemetry import initialize_opentelemetry_tracer_provider
from phoenix.trace.schemas import Span
from phoenix.utilities.client import PHOENIX_SERVER_VERSION_HEADER
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid this import to keep server and client easily separable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's easy to separate either way, it's a constant, this is just a way to coordinate on the name. I'd argue that on the hierarchy of couplings we need untangle when making new packages a single name import has never made separating work into a package more or less difficult


if TYPE_CHECKING:
from opentelemetry.trace import TracerProvider
Expand Down Expand Up @@ -167,8 +168,11 @@ async def dispatch(
request: Request,
call_next: RequestResponseEndpoint,
) -> Response:
from phoenix import __version__ as phoenix_version
RogerHYang marked this conversation as resolved.
Show resolved Hide resolved

response = await call_next(request)
response.headers["x-colab-notebook-cache-control"] = "no-cache"
response.headers[PHOENIX_SERVER_VERSION_HEADER] = phoenix_version
return response


Expand Down
3 changes: 2 additions & 1 deletion src/phoenix/session/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
from phoenix.trace import Evaluations, TraceDataset
from phoenix.trace.dsl import SpanQuery
from phoenix.trace.otel import encode_span_to_otlp
from phoenix.utilities.client import VersionedClient

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -92,7 +93,7 @@ def __init__(
host = "127.0.0.1"
base_url = endpoint or get_env_collector_endpoint() or f"http://{host}:{get_env_port()}"
self._base_url = base_url if base_url.endswith("/") else base_url + "/"
self._client = httpx.Client(headers=headers)
self._client = VersionedClient(headers=headers)
weakref.finalize(self, self._client.close)
if warn_if_server_not_running:
self._warn_if_phoenix_is_not_running()
Expand Down
82 changes: 82 additions & 0 deletions src/phoenix/utilities/client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import warnings
from typing import Any

import httpx

PHOENIX_SERVER_VERSION_HEADER = "x-phoenix-server-version"


class VersionedClient(httpx.Client):
"""
A httpx.Client wrapper that warns if there is a server/client version mismatch.
"""

def __init__(self, *args: Any, **kwargs: Any):
from phoenix import __version__ as phoenix_version

super().__init__(*args, **kwargs)
self._client_phoenix_version = phoenix_version
self._major, self._minor, self._patch = map(int, self._client_phoenix_version.split("."))
anticorrelator marked this conversation as resolved.
Show resolved Hide resolved
self._warned_on_minor_version_mismatch = False

def _check_version(self, response: httpx.Response) -> None:
server_version = response.headers.get(PHOENIX_SERVER_VERSION_HEADER)
if server_version is None:
return
anticorrelator marked this conversation as resolved.
Show resolved Hide resolved

server_major, server_minor, server_patch = map(int, server_version.split("."))
anticorrelator marked this conversation as resolved.
Show resolved Hide resolved
if abs(server_major - self._major) >= 1:
warnings.warn(
f"⚠️⚠️ The Phoenix server ({server_version}) and client "
f"({self._client_phoenix_version}) versions are severely mismatched. Upgrade "
" either the client or server to ensure API compatibility ⚠️⚠️"
)
elif abs(server_minor - self._minor) >= 1 and not self._warned_on_minor_version_mismatch:
self._warned_on_minor_version_mismatch = True
RogerHYang marked this conversation as resolved.
Show resolved Hide resolved
warnings.warn(
f"The Phoenix server ({server_version}) and client ({self._client_phoenix_version})"
" versions are mismatched and may have compatibility issues."
)

def request(self, *args: Any, **kwargs: Any) -> httpx.Response:
response = super().request(*args, **kwargs)
self._check_version(response)
return response


class VersionedAsyncClient(httpx.AsyncClient):
"""
A httpx.Client wrapper that warns if there is a server/client version mismatch.
"""

def __init__(self, *args: Any, **kwargs: Any):
from phoenix import __version__ as phoenix_version

super().__init__(*args, **kwargs)
self._client_phoenix_version = phoenix_version
self._major, self._minor, self._patch = map(int, self._client_phoenix_version.split("."))
self._warned_on_minor_version_mismatch = False

async def _check_version(self, response: httpx.Response) -> None:
anticorrelator marked this conversation as resolved.
Show resolved Hide resolved
server_version = response.headers.get(PHOENIX_SERVER_VERSION_HEADER)
if server_version is None:
return

server_major, server_minor, server_patch = map(int, server_version.split("."))
if abs(server_major - self._major) >= 1:
warnings.warn(
f"⚠️⚠️ The Phoenix server ({server_version}) and client "
f"({self._client_phoenix_version}) versions are severely mismatched. Upgrade "
" either the client or server to ensure API compatibility ⚠️⚠️"
)
elif abs(server_minor - self._minor) >= 1 and not self._warned_on_minor_version_mismatch:
self._warned_on_minor_version_mismatch = True
warnings.warn(
f"The Phoenix server ({server_version}) and client ({self._client_phoenix_version})"
" versions are mismatched and may have compatibility issues."
)

async def request(self, *args: Any, **kwargs: Any) -> httpx.Response:
response = await super().request(*args, **kwargs)
await self._check_version(response)
return response
Loading