Add native run binding for Workers AI by G4brym · Pull Request #5371 · cloudflare/workers-sdk

G4brym · 2024-03-25T17:51:25Z

What this PR solves / how to test

Adds native binding for workers ai.
CR-852557

Author has addressed the following

Tests
- TODO (before merge)
- Included
- Not necessary because:
Changeset (Changeset guidelines)
- TODO (before merge)
- Included
- Not necessary because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/...
- Not necessary because: will be documented post-release. Users can continue to use the old interface for now.

changeset-bot · 2024-03-25T17:51:29Z

🦋 Changeset detected

Latest commit: c49bf87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
wrangler	Minor
@cloudflare/vitest-pool-workers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-03-25T19:14:43Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-wrangler-5371

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5371/npm-package-wrangler-5371

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-wrangler-5371 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-create-cloudflare-5371 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-cloudflare-kv-asset-handler-5371

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-miniflare-5371

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-cloudflare-pages-shared-5371

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8498132672/npm-package-cloudflare-vitest-pool-workers-5371

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.41.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240320.1
`workerd`	1.20240329.0	1.20240329.0
`workerd --version`	1.20240329.0	2024-03-29

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

admah · 2024-03-26T14:27:16Z

Will this change the user experience for local dev?

G4brym · 2024-03-26T14:38:52Z

@admah yes, user will be able to use Workers ai without installing the @cloudflare/ai npm package
but i've kept the .fetch() method, so anyone using the "soon deprecated" npm packge will still be able to run models without changing anything in ther code

irvinebroque · 2024-03-27T18:52:54Z

@G4brym — assume this need to go out in wrangler for next week? ready for review? Think @craigsdennis trying to build something using new binding

G4brym · 2024-03-30T15:10:17Z

@irvinebroque only now i could finish the pr, because workerd was only released this morning
this pr is ready, but with the new workerd release, some queue's tests are now failing

G4brym · 2024-03-30T15:14:31Z

I'm not sure if miniflare needs to be released first, as this wrangler changes depends on a miniflare release

irvinebroque · 2024-03-30T22:15:02Z

Think PR description would help here as took a sec to wrap my head around why this needed to refactor bit of Miniflare.

Tests fail only with your changes or also with latest workerd? Have you dug in? Bumped here to test: #5451

  [mf:wrn] Dropped message "1df5e791450c89d5864f9204b6094d8e" on queue "queue" after 3 failed attempts!
   ❯ |queues| test/queue-producer-unit.test.ts  (2 tests | 1 failed) 1101ms
     ❯ test/queue-producer-unit.test.ts > produces queue message with mocked consumer
       → Timed out in waitUntil!
   ❯ |queues| test/queue-consumer-integration-self.test.ts  (1 test | 1 failed) 78ms
     ❯ test/queue-consumer-integration-self.test.ts > consumes queue messages
       → The value cannot be converted because it is not an integer.
   ✓ |queues| test/queue-consumer-unit.test.ts  (1 test) 68ms

    index › Miniflare: getWorker() allows dispatching events directly
  
    test/index.spec.ts:1177
  
     1176:   // Check `Fetcher#queue()`                          
     1177:   let queueResult = await fetcher.queue("needy", [    
     1178:     { id: "a", timestamp: new Date(1000), body: "a" },
  
    Rejected promise returned by test. Reason:
  
    TypeError {
      message: 'The value cannot be converted because it is not an integer.',
    }

https://github.com/cloudflare/workers-sdk/blob/main/packages/miniflare/test/index.spec.ts#L1177

https://github.com/cloudflare/workers-sdk/blob/main/packages/miniflare/test/index.spec.ts#L19

https://github.com/cloudflare/workers-sdk/blob/main/packages/miniflare/test/index.spec.ts#L1156

irvinebroque · 2024-03-30T22:22:22Z

Tests fail without the Miniflare changes, just when bumping workerd to latest.

irvinebroque · 2024-03-30T22:24:47Z

Believe cloudflare/workerd@5a4beaa is the only Queues related commit that went out in latest workerd release.

@sesteves — not totally sure if cause, but wanted to flag

mrbbot · 2024-03-31T09:36:14Z

#5455 should bump workerd and address the failures we're seeing here. Once that's merged, we can rebase this PR on top of that. 👍

mrbbot · 2024-03-31T10:06:24Z

Suggested change

wrappedBindings?: Record<string, object>;

mrbbot · 2024-03-31T10:06:30Z

Suggested change

| "wrappedBindings"

...but add this to the Pick for WorkerOptionsBindings above.

mrbbot · 2024-03-31T10:08:38Z

Ideally, we shouldn't be mutating the config object passed in here. Instead, I would construct an empty wrappedBindings object above this if block with the type WorkerOptions["wrappedBindings"], then modify that here if AI is enabled:

const wrappedBindings: WorkerOptions["wrappedBindings"] = {}; if (bindings.ai?.binding) { // ... wrappedBindings[bindings.ai.binding] = { scriptName: EXTERNAL_AI_WORKER_NAME, }; }

Agreed and done

codecov · 2024-03-31T11:45:24Z

Codecov Report

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

Project coverage is 72.27%. Comparing base (d994066) to head (c49bf87).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5371      +/-   ##
==========================================
+ Coverage   72.15%   72.27%   +0.12%     
==========================================
  Files         326      326              
  Lines       16848    16853       +5     
  Branches     4311     4311              
==========================================
+ Hits        12156    12180      +24     
+ Misses       4692     4673      -19

Files	Coverage Δ
packages/wrangler/src/ai/fetcher.ts	`35.71% <66.66%> (+10.71%)`	⬆️
...es/wrangler/src/api/integrations/platform/index.ts	`16.66% <0.00%> (ø)`
packages/wrangler/src/dev/miniflare.ts	`63.43% <66.66%> (+1.82%)`	⬆️

... and 8 files with indirect coverage changes

G4brym · 2024-03-31T11:57:32Z

Thanks, i've rebased the pr, implemented the suggestions and tests are now passing 👍

Co-authored-by: MrBBot <me@mrbbot.dev>

G4brym · 2024-03-31T21:06:09Z

Hey @mrbbot i don't have permissions to merge this pr, can you merge it please?

G4brym marked this pull request as ready for review March 25, 2024 18:56

G4brym requested a review from a team as a code owner March 25, 2024 18:56

G4brym changed the title ~~WIP: Improve workers ai binding using wrappedBindings~~ Improve workers ai binding using wrappedBindings Mar 25, 2024

G4brym changed the title ~~Improve workers ai binding using wrappedBindings~~ Add native run binding for Workers AI Mar 25, 2024

G4brym mentioned this pull request Mar 26, 2024

Add Workers Ai fetcher api cloudflare/workerd#1877

Merged

mrbbot reviewed Mar 31, 2024

View reviewed changes

Improve workers ai binding using wrappedBindings

a774a97

Update .changeset/hot-geese-deliver.md

c49bf87

Co-authored-by: MrBBot <me@mrbbot.dev>

mrbbot approved these changes Mar 31, 2024

View reviewed changes

mrbbot merged commit 77152f3 into cloudflare:main Apr 1, 2024

mrbbot added the ldw-exempt PRs marked with this label can be included in releases during LDW. label Apr 1, 2024

workers-devprod mentioned this pull request Apr 1, 2024

Version Packages #5457

Merged

Conversation

G4brym commented Mar 25, 2024 • edited by penalosa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR solves / how to test

Author has addressed the following

Uh oh!

changeset-bot Bot commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

admah commented Mar 26, 2024

Uh oh!

G4brym commented Mar 26, 2024

Uh oh!

irvinebroque commented Mar 27, 2024

Uh oh!

G4brym commented Mar 30, 2024

Uh oh!

G4brym commented Mar 30, 2024

Uh oh!

irvinebroque commented Mar 30, 2024

Uh oh!

irvinebroque commented Mar 30, 2024

Uh oh!

irvinebroque commented Mar 30, 2024

Uh oh!

mrbbot commented Mar 31, 2024

Uh oh!

Uh oh!

mrbbot Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

G4brym Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

mrbbot Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

G4brym Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

mrbbot Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

G4brym Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

G4brym commented Mar 31, 2024

Uh oh!

G4brym commented Mar 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

G4brym commented Mar 25, 2024 •

edited by penalosa

Loading

changeset-bot Bot commented Mar 25, 2024 •

edited

Loading

github-actions Bot commented Mar 25, 2024 •

edited

Loading

codecov Bot commented Mar 31, 2024 •

edited

Loading