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

Add support for Workers AI in local mode #4522

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

G4brym
Copy link
Member

@G4brym G4brym commented Nov 29, 2023

Fixes AI-380

What this PR solves / how to test:
Adds support for Workers AI in local mode.
To test this run an example worker from the Workers AI Docs run wrangler in local mode and the script should be able to execute successfully.

Author has addressed the following:

Note for PR author:

We want to celebrate and highlight awesome PR review! If you think this PR received a particularly high-caliber review, please assign it the label highlight pr review so future reviewers can take inspiration and learn from it.

@G4brym G4brym requested a review from a team as a code owner November 29, 2023 16:22
Copy link

changeset-bot bot commented Nov 29, 2023

🦋 Changeset detected

Latest commit: 8e79be9

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

This PR includes changesets to release 1 package
Name Type
wrangler Minor

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

Copy link
Contributor

github-actions bot commented Nov 29, 2023

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/7103899088/npm-package-wrangler-4522

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7103899088/npm-package-wrangler-4522 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7103899088/npm-package-miniflare-4522
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7103899088/npm-package-cloudflare-pages-shared-4522
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7103899088/npm-package-create-cloudflare-4522

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


wrangler@3.18.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20231030.2
workerd 1.20231030.0 1.20231030.0
workerd --version 1.20231030.0 2023-10-30

|

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

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

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

Nice! 🎉 Added a few comments...

packages/wrangler/src/ai/fetcher.ts Outdated Show resolved Hide resolved
Comment on lines 1 to 2
import {getAccountId} from "../user";
import {performApiFetch} from "../cfetch/internal";
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll likely need to format your code for checks to pass. Run pnpm run prettify in the root of the repository.

Copy link
Member Author

Choose a reason for hiding this comment

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

code formated

serviceBindings: config.serviceBindings,
serviceBindings: {
...config.serviceBindings,
AI: AIFetcher
Copy link
Contributor

Choose a reason for hiding this comment

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

We should only be adding an AI binding if it's enabled. The binding may not be bound as AI too. See bindings.ai for the required configuration. If bindings.ai === undefined, this binding shouldn't be set. If it's defined, AIFetcher should be bound as bindings.ai.binding.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed the issue, please take a look now

"Workers AI is not currently supported in local mode. Please use --remote to work with it."
);
}

if (!config.bindings.ai && config.bindings.vectorize?.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to remove this check too, so we still warn that Vectorize bindings are unsupported if AI bindings are enabled.

Suggested change
if (!config.bindings.ai && config.bindings.vectorize?.length) {
if (config.bindings.vectorize?.length) {

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@mrbbot
Copy link
Contributor

mrbbot commented Nov 29, 2023

For testing this, I'd probably copy fixtures/worker-app to fixtures/ai-app, and edit the export default { fetch(...) { ... } } handler to verify that a service binding with the correct name is bound to the worker. If you actually wanted to test AI functionality worked, you could consider adding an E2E test here: https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/e2e/dev.test.ts. These run with an actual Cloudflare API tokens so would be able to access the AI proxy.

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #4522 (8e79be9) into main (1b34878) will increase coverage by 0.02%.
Report is 16 commits behind head on main.
The diff coverage is 46.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4522      +/-   ##
==========================================
+ Coverage   75.46%   75.48%   +0.02%     
==========================================
  Files         240      241       +1     
  Lines       12851    12867      +16     
  Branches     3309     3315       +6     
==========================================
+ Hits         9698     9713      +15     
- Misses       3153     3154       +1     
Files Coverage Δ
packages/wrangler/src/api/dev.ts 82.85% <ø> (ø)
packages/wrangler/src/dev.tsx 82.07% <100.00%> (+0.07%) ⬆️
packages/wrangler/src/dev/miniflare.ts 62.50% <75.00%> (+2.59%) ⬆️
packages/wrangler/src/pages/dev.ts 16.43% <0.00%> (+0.23%) ⬆️
packages/wrangler/src/ai/fetcher.ts 33.33% <33.33%> (ø)

... and 6 files with indirect coverage changes

@G4brym G4brym force-pushed the add-support-for-workers-ai-locally branch 4 times, most recently from 290fc97 to 8b91f7d Compare December 4, 2023 10:41
@G4brym G4brym force-pushed the add-support-for-workers-ai-locally branch from 14b20b1 to 1ac3363 Compare December 4, 2023 15:03
@G4brym
Copy link
Member Author

G4brym commented Dec 4, 2023

Hey @mrbbot can you take a look at this pr again?
I've added a simple e2e test and your recommendations

@G4brym
Copy link
Member Author

G4brym commented Dec 5, 2023

@mrbbot all builds are now passing, can you merge this and give an approximate timeline for release?

@G4brym G4brym requested a review from a team as a code owner December 5, 2023 16:15
@mrbbot mrbbot merged commit c10bf0f into cloudflare:main Dec 6, 2023
17 checks passed
@workers-devprod workers-devprod mentioned this pull request Dec 6, 2023
@acusti
Copy link
Contributor

acusti commented Dec 12, 2023

@G4brym @mrbbot this is great, thanks so much for working on it! i just upgraded to v3.20.0 to try it out, but when i try to use my worker that has AI bindings locally, i get the following error:

InferenceUpstreamError: {"success":false,"errors":[{"code":10000,"message":"Authentication error"}]}

      at InferenceSession.run (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:34888:13)
      at async Ai.run (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:34932:70)
      at async loader (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:34950:16)
      at async callRouteLoaderRR (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:3762:16)
      at async callLoaderOrAction (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:2851:16)
      at async Promise.all (index 0)
      at async loadRouteData (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:2555:19)
      at async queryImpl (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:2432:20)
      at async Object.queryRoute (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:2413:18)
      at async handleResourceRequestRR (file:///Users/path/to/.wrangler/tmp/dev-lzzKC2/index.js:3990:20) {
    httpCode: 403

my invocation looks like:

    const ai = new Ai(context.env.AI);

    // https://developers.cloudflare.com/workers-ai/models/text-generation/#available-embedding-models
    const stream = await ai.run('@cf/meta/llama-2-7b-chat-int8', {
        messages: [
            { content: PROMPT_CONTEXT, role: 'system' },
            { content: prompt, role: 'user' },
        ],
        stream: true,
    });

    return new Response(stream, {
        headers: { 'content-type': 'text/event-stream' },
    });

note that the above code works 100% when running wrangler with the --remote flag, both with wrangler v3.20.0 and v3.19.0.

@G4brym
Copy link
Member Author

G4brym commented Dec 13, 2023

Hey @acusti can you run wrangler logout and then wrangler login to check if the issue is solved?
Thanks for reporting this

@acusti
Copy link
Contributor

acusti commented Dec 13, 2023

@G4brym that worked! thanks so much for the quick solution, and sorry for the trouble.

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