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

Support backwards compatibility with environment names and related CLI flags #680

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

JacobMGEvans
Copy link
Contributor

  1. When in Legacy environment mode we should not compute name field if specified in an environment.
  2. Throw an Error when --env and --name are used together in Legacy Environment, except for Secrets & Tail which are using a special case getLegacyScriptName for parity with Wrangler1
  3. Started the refactor for args being utilized at the Config level, currently checking for Legacy Environment only.

Fixes #672

@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2022

🦋 Changeset detected

Latest commit: 50cfa2b

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

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

…related CLI flags

1. When in Legacy environment mode we should not compute name field if specified in an environment.
2. Throw an Error when `--env` and `--name` are used together in Legacy Environment, except for Secrets & Tail which are using a special case `getLegacyScriptName` for parity with Wrangler1
3. Started the refactor for args being utilized at the Config level, currently checking for Legacy Environment only.

Fixes #672
@petebacondarwin petebacondarwin merged commit 8e2cbaf into main Mar 23, 2022
@petebacondarwin petebacondarwin deleted the jacobmgevans/legacy-env-name branch March 23, 2022 16:49
@github-actions github-actions bot mentioned this pull request Mar 23, 2022
@github-actions
Copy link
Contributor

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

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2029542841/npm-package-wrangler-680

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

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/680/npm-package-wrangler-680

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2029542841/npm-package-wrangler-680 dev path/to/script.js

@threepointone
Copy link
Contributor

This is breaking environments right now. In legacy mode, names do not have -<environment> appended anymore to script names, which makes publishes publish directly to the 'production'/default name.

petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this pull request Mar 28, 2022
When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See cloudflare#680 (comment)
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this pull request Mar 28, 2022
When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See cloudflare#680 (comment)
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this pull request Mar 28, 2022
When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See cloudflare#680 (comment)
threepointone pushed a commit that referenced this pull request Mar 28, 2022
…ts (#719)

When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See #680 (comment)
mrbbot added a commit that referenced this pull request Oct 31, 2023
We previously waited for Miniflare to be ready before `dispose()`ing.
Unfortunately, we weren't waiting for the `workerd` config to finish
being written to stdin. Calling `dispose()` immediately after
`new Miniflare()` would stop waiting for socket ports to be reported,
and kill the `workerd` process while data was still being written.
This threw an unhandled `EPIPE` error.

This changes makes sure we don't report that Miniflare is ready until
after the config is fully-written.

Closes #680
mrbbot added a commit that referenced this pull request Nov 1, 2023
We previously waited for Miniflare to be ready before `dispose()`ing.
Unfortunately, we weren't waiting for the `workerd` config to finish
being written to stdin. Calling `dispose()` immediately after
`new Miniflare()` would stop waiting for socket ports to be reported,
and kill the `workerd` process while data was still being written.
This threw an unhandled `EPIPE` error.

This changes makes sure we don't report that Miniflare is ready until
after the config is fully-written.

Closes #680
mrbbot added a commit that referenced this pull request Nov 1, 2023
We previously waited for Miniflare to be ready before `dispose()`ing.
Unfortunately, we weren't waiting for the `workerd` config to finish
being written to stdin. Calling `dispose()` immediately after
`new Miniflare()` would stop waiting for socket ports to be reported,
and kill the `workerd` process while data was still being written.
This threw an unhandled `EPIPE` error.

This changes makes sure we don't report that Miniflare is ready until
after the config is fully-written.

Closes #680
mrbbot added a commit that referenced this pull request Nov 1, 2023
We previously waited for Miniflare to be ready before `dispose()`ing.
Unfortunately, we weren't waiting for the `workerd` config to finish
being written to stdin. Calling `dispose()` immediately after
`new Miniflare()` would stop waiting for socket ports to be reported,
and kill the `workerd` process while data was still being written.
This threw an unhandled `EPIPE` error.

This changes makes sure we don't report that Miniflare is ready until
after the config is fully-written.

Closes #680
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.

🐛 BUG: Legacy Env name computed on publish
3 participants