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

Fix ai binding logs property #2095

Merged
merged 1 commit into from
May 9, 2024

Conversation

G4brym
Copy link
Member

@G4brym G4brym commented May 8, 2024

This re-adds a removed property as backwards compatibility, removed in the last release

@G4brym G4brym requested review from a team as code owners May 8, 2024 14:51
@G4brym G4brym force-pushed the fix-ai-binding-logs-property branch 2 times, most recently from 8cd6b82 to d8cc88e Compare May 8, 2024 15:05
@DaniFoldi
Copy link
Contributor

Hi, I saw the workers-sdk test broken by the removal of this property. Just curious, if you can share, is there any particular reason the logs were removed/hidden in the first place?

Something like AI is much more complicated than say KV - there's not much to log in the case of KV (put or delete, that's it), but with AI, many things can happen, and logs were useful for me (and probably other folks who found them).

@G4brym G4brym force-pushed the fix-ai-binding-logs-property branch from d8cc88e to 232e061 Compare May 8, 2024 15:22
@G4brym
Copy link
Member Author

G4brym commented May 8, 2024

Hey @DaniFoldi logs are not structured, and we change their format all the time
Since the beginning it was meant to be used just internally by the team to debug some models, and we have now moved on to a better logging solution
We are just not in a position currently to guarantee that the logs will keep the same structure going forward, and because we cannot guarantee their format it's better to not expose them as users, like you, might start relying on them
Maybe in the future, when and if we add custom models we might bring back logs in an official format

@RamIdeas
Copy link
Contributor

RamIdeas commented May 9, 2024

FYI we've decided to hold off upgrading workerd in wrangler until this PR lands and is released cloudflare/workers-sdk#5752 (comment)

@RamIdeas RamIdeas merged commit 62481a4 into cloudflare:main May 9, 2024
10 checks passed
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.

4 participants