feat: Support basic auth #3061
feat: Support basic auth #3061
Conversation
dosubot
bot
added
the
size:XS
src/phoenix/server/main.py
Outdated
| @@ -189,6 +189,7 @@ def _get_pid_file() -> Path: | |||
|
|
|||
| # 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. | |||
| display_host = host | |||
There was a problem hiding this comment.
@anticorrelator this was broken I think
| const graphQLPath = | ||
| `${window.location.protocol}//${window.location.host}${getSanitizedPath(window.Config.basename)}` + | ||
| "/graphql"; |
There was a problem hiding this comment.
this is needed because basic auth comes in as http://{user}:{pass}asdfas.com but this needs to be stripped from the graphQL path
src/phoenix/server/main.py
Outdated
| display_host = host | ||
| if ":" in host: | ||
| # format IPv6 hosts in brackets | ||
| display_host = f"[{host}]" |
There was a problem hiding this comment.
Was mypy not catching this?
There was a problem hiding this comment.
I think we are merging without checking CI?
dosubot
bot
added
size:S
| @@ -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 | |||
There was a problem hiding this comment.
filed an issue @anticorrelator - this seems strange to force a none type in here: #3062
There was a problem hiding this comment.
Here's the context behind this solution that Dustin had found: encode/uvicorn#1529 (reply in thread)
There was a problem hiding this comment.
@anticorrelator could we add a comment linking this issue?
* feat: add the ability to use basic auth * mypy * force fix
Changes the url for graphql so that basic auth can be properly initialized