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

Improve the OpenAI integration #104

Merged
merged 3 commits into from
May 3, 2024
Merged

Improve the OpenAI integration #104

merged 3 commits into from
May 3, 2024

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented May 3, 2024

Copy link

cloudflare-workers-and-pages bot commented May 3, 2024

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: d84f8fd
Status: ✅  Deploy successful!
Preview URL: https://c44cccd4.logfire-docs.pages.dev
Branch Preview URL: https://add-type-hints-on-openai.logfire-docs.pages.dev

View logs

@Kludex Kludex requested a review from samuelcolvin May 3, 2024 11:40
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 97.61905% with 1 lines in your changes are missing coverage. Please review.

Files Patch % Lines
logfire/_internal/integrations/openai.py 97.61% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM


from ..main import Logfire, LogfireSpan

_AsyncStreamT = TypeVar('_AsyncStreamT', bound=AsyncStream[Any])
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment explaining roughly what these do and why they're needed - just one comment for all of them is probably enough.

logfire/_internal/integrations/openai.py Outdated Show resolved Hide resolved
@alexmojaki
Copy link
Contributor

Does this take #54 into account?

@Kludex Kludex closed this May 3, 2024
@Kludex Kludex reopened this May 3, 2024
@Kludex Kludex force-pushed the add-type-hints-on-openai branch from ddaa2db to f76ef10 Compare May 3, 2024 12:40
@Kludex
Copy link
Member Author

Kludex commented May 3, 2024

Does this take #54 into account?

No. Unrelated.

@Kludex Kludex enabled auto-merge (squash) May 3, 2024 12:41
@Kludex Kludex merged commit 944c590 into main May 3, 2024
11 checks passed
@Kludex Kludex deleted the add-type-hints-on-openai branch May 3, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAI SDK traces will fail when .with_raw_response is used
3 participants