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

Experimental worker-to-worker bindings support #166

Merged
merged 6 commits into from
Dec 24, 2021
Merged

Conversation

threepointone
Copy link
Contributor

@threepointone threepointone commented Dec 23, 2021

Adds support for experimental worker-to-worker bindings, letting you call a worker from another worker. The Wrangler config field looks like follows:

experimental_services = [{ name = "foo", service = "foo-service", environment = "production" }]

Where:

  • name is the name of the binding in your worker code
  • service is the service to reference
  • environment is the environment of the script to reference (e.g. production, staging, etc)

What doesn't work?

  • The API still doesn't support worker-to-worker bindings for previews, so wrangler dev will fail for now

Differences from #156

This PR is based in #156, with a few changes -

  • the configuration field env is renamed to environment (we want to move away from short forms like env)
  • the configuration field environment is not an optional field (because we want to explicitly bind to environments, or folks could accidentally mess with production data)
  • the configuration field services is called experimental_services so it's clear on usage that this is not ready for production usage yet, and is only for our internal testing. We also log a warning when we see the field.

@changeset-bot
Copy link

changeset-bot bot commented Dec 23, 2021

🦋 Changeset detected

Latest commit: c6408f9

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

This PR includes changesets to release 1 package
Name Type
wrangler 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

packages/wrangler/src/api/worker.ts Outdated Show resolved Hide resolved
Base automatically changed from refactor-bindings to main December 23, 2021 20:08
There are 3 types of 'bindings' that we upload with a worker definition: `vars`, `kv_namespaces`, and `durable_objects`. These are configured in `wrangler.toml`, and get passed through to the api that uploads a worker definition (in `form_data.ts`), when using `dev` and `publish` commands.

We currently serialise it as a big `variables` object, where the key of the objects defines the binding "name", and the value contains information to be bound. (https://github.com/cloudflare/wrangler2/blob/0330ecf1b54c92dfe86cb3f38394f453ed418381/packages/wrangler/src/index.tsx#L507-L530) This code sucks for many reasons: it loses some type information, hard to read/grok, hard to add new types of bindings, and mostly it's unnecessary.

This PR refactors this code to instead mostly preserve the configuration style structure all the way until we actually serialise it when uploading a worker definition. I also added some more types along the way, and cleared some dead code.

I will follow up this PR with 2 PRs soon: one, which introduces service bindings (as a fresh take on #156), and another one that introduces tests for configuration.
Adds support for experimental worker-to-worker bindings, letting you call a worker from another worker. The Wrangler config field looks like follows:

```toml
services = [{ name = "foo", script_name = "foo-service", environment = "production" }]
```
Where:
- `name` is the name of the binding in your worker code
- `script_name` is the script to reference
- `environment` is the environment of the script to reference (e.g. `production`, `staging`, etc)

### What doesn't work?

- The API still doesn't support worker-to-worker bindings for previews, so `wrangler dev` will fail for now

### Differences from #156

This PR is based in #156, with a few changes -

- the configuration field `env` is renamed to `environment` (we want to move away from short forms like `env`)
- the configuration field `environment` is not an optional field (because we want to explicitly bind to environments, or folks could accidentally mess with production data)
- the configuration field services is called `experimental_services` so it's clear on usage that this is not ready for production usage yet, and is only for our internal testing. We also log a warning when we see the field.
@threepointone threepointone merged commit ce2d7d1 into main Dec 24, 2021
@threepointone threepointone deleted the service-bindings branch December 24, 2021 00:54
@danielrs
Copy link
Contributor

danielrs commented Jan 3, 2022

Opened a PR in Wrangler V1 for the same feature:
cloudflare/wrangler-legacy#2173

@ObsidianMinor
Copy link
Contributor

ObsidianMinor commented Jan 3, 2022

Missed this when it was originally written. I'm confused why we are naming the field experimental_services instead of just services? Odds are the interface won't change, but by naming the field this we're forcing everyone downstream and ourselves as maintainers to come back and update the field name when it officially releases. What's the point?

@threepointone
Copy link
Contributor Author

This is not ready for usage by customers just yet, and it's not even ready as a feature for interns usage yet. Marking it as experimental makes it obvious. When we do it close it to everyone, we can do it in two steps where we deprecate the experimental_ prefix in one version with a warning and remove it in the next.

@ObsidianMinor
Copy link
Contributor

But normal users can't make Workers with service bindings anyway. You need a feature flag enabled on your account. To do that you need to talk to someone that can give you the flag and they will probably tell you about the limitations of its current state. So those that have access likely already know that it's in internal beta and already know of its limitations. Still seems like unnecessary complexity...

@threepointone
Copy link
Contributor Author

That's fine, we'll absorb the extra complexity to do it right. It's not that much.

GregBrimble pushed a commit that referenced this pull request Feb 8, 2024
* defined response status on cached responses. fixes 165

* updated mocks and added tests for range requests

* fixed range request test
GregBrimble pushed a commit that referenced this pull request Feb 9, 2024
* defined response status on cached responses. fixes 165

* updated mocks and added tests for range requests

* fixed range request test
GregBrimble pushed a commit that referenced this pull request Feb 9, 2024
* defined response status on cached responses. fixes 165

* updated mocks and added tests for range requests

* fixed range request test
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