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: Support basic auth #3061

Merged
merged 3 commits into from
May 3, 2024
Merged
Changes from all 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: 4 additions & 1 deletion app/src/RelayEnvironment.ts
Original file line number Diff line number Diff line change
@@ -9,7 +9,10 @@ import {
function getSanitizedPath(path: string): string {
return path.endsWith("/") ? path.slice(0, -1) : path;
}
const graphQLPath = getSanitizedPath(window.Config.basename) + "/graphql";
const graphQLPath =
`${window.location.protocol}//${window.location.host}${getSanitizedPath(window.Config.basename)}` +
"/graphql";
Comment on lines +12 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is needed because basic auth comes in as http://{user}:{pass}asdfas.com but this needs to be stripped from the graphQL path


/**
* Relay requires developers to configure a "fetch" function that tells Relay how to load
* the results of GraphQL queries from your server (or other data source). See more at
9 changes: 5 additions & 4 deletions src/phoenix/server/main.py
Original file line number Diff line number Diff line change
@@ -185,14 +185,15 @@ def _get_pid_file() -> Path:
trace_dataset_name = args.trace_fixture
simulate_streaming = args.simulate_streaming

host = args.host or get_env_host()

host: Optional[str] = args.host or get_env_host()
display_host = host or "localhost"
# If the host is "::", the convention is to bind to all interfaces. However, uvicorn
# does not support this directly unless the host is set to None.
if ":" in host:
if host and ":" in host:
# format IPv6 hosts in brackets
display_host = f"[{host}]"
if host == "::":
# TODO(dustin): why is this necessary? it's not type compliant
host = None

port = args.port or get_env_port()
@@ -250,7 +251,7 @@ def _get_pid_file() -> Path:
initial_spans=fixture_spans,
initial_evaluations=fixture_evals,
)
server = Server(config=Config(app, host=host, port=port))
server = Server(config=Config(app, host=host, port=port)) # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

filed an issue @anticorrelator - this seems strange to force a none type in here: #3062

Copy link
Contributor

@RogerHYang RogerHYang May 3, 2024

Choose a reason for hiding this comment

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

Here's the context behind this solution that Dustin had found: encode/uvicorn#1529 (reply in thread)

Copy link
Contributor

Choose a reason for hiding this comment

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

@anticorrelator could we add a comment linking this issue?

Thread(target=_write_pid_file_when_ready, args=(server,), daemon=True).start()

# Print information about the server