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

core: Run LangChainTracer inline #26797

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Conversation

nfcampos
Copy link
Collaborator

@nfcampos nfcampos commented Sep 24, 2024

  • this flag ensures the tracer always runs in the same thread as the run being traced for both sync and async runs
  • pro: less chance for ordering bugs and other oddities
  • blocking the event loop is not a concern given all code in the tracer holds the GIL anyway

- this flag ensures the tracer always runs in the same thread as the run being traced for both sync and async runs
- pro: less chance for ordering bugs and other oddities
- blocking the event loop is not a concern given all code in the tracer holds the GIL anyway
Copy link

vercel bot commented Sep 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 0:49am

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. Ɑ: core Related to langchain-core 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs labels Sep 24, 2024
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Sep 24, 2024
@eyurtsev
Copy link
Collaborator

Aren't there network requests being done which would result in the gil being released?

@nfcampos
Copy link
Collaborator Author

The langchain tracer doesn't make any network requests inline

@eyurtsev
Copy link
Collaborator

Got it -- won't hurt to add a doc-string in run_inline explaining why that's OK. Knowing that this is OK depends on understanding what's happening inside the langsmith client

@nfcampos nfcampos merged commit de70a64 into master Sep 24, 2024
98 checks passed
@nfcampos nfcampos deleted the nc/23sep/langchain-tracer-inline branch September 24, 2024 15:31
Sheepsta300 pushed a commit to Sheepsta300/langchain that referenced this pull request Oct 1, 2024
- this flag ensures the tracer always runs in the same thread as the run
being traced for both sync and async runs
- pro: less chance for ordering bugs and other oddities
- blocking the event loop is not a concern given all code in the tracer
holds the GIL anyway
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ɑ: core Related to langchain-core lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants
  翻译: