-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Assistant] Fixes LangSmith support for specifying configuration via env vars #180426
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
@@ -124,7 +124,7 @@ export const getLangSmithTracer = ({ | |||
return []; | |||
} | |||
const lcTracer = new LangChainTracer({ | |||
projectName: projectName ?? 'default', // Shows as the 'test' run's 'name' in langsmith ui | |||
projectName, // Shows as the 'test' run's 'name' in langsmith ui |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine to pass in undefined
as the projectName here. LangChain will attempt to fetch it from the env vars, and if still not available it will then fall back to default
when pushing to LangSmith.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this fixed quickly. Ran your test and had success, LGTM!
💚 Build Succeeded
Metrics [docs]Async chunks
To update your PR or re-run it, just comment with: cc @spong |
Summary
With #180227, LangSmith configuration (Project & API Key) could no longer be specified using environment variables when working locally. This fixes that issue, which was caused by sending
''
forlangSmithProject
andlangSmithApiKey
instead ofundefined
.To test, set the below env vars, then start kibana. Be sure to not have the UI trace options set as shown in #180227.