-
Notifications
You must be signed in to change notification settings - Fork 688
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: use account id for listing zones #6164
Conversation
🦋 Changeset detectedLatest commit: cdcb94e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
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/9730503953/npm-package-wrangler-6164 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6164/npm-package-wrangler-6164 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-wrangler-6164 dev path/to/script.js Additional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-create-cloudflare-6164 --no-auto-update npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-cloudflare-kv-asset-handler-6164 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-miniflare-6164 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-cloudflare-pages-shared-6164 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9730503953/npm-package-cloudflare-vitest-pool-workers-6164 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
3dafbf4
to
8c5b227
Compare
c73dc47
to
1840b9d
Compare
@@ -85,7 +85,7 @@ export async function versionsRollbackHandler(args: VersionsRollbackArgs) { | |||
})); | |||
|
|||
const message = await prompt( | |||
"Please provide an optional message for this rollback (120 characters max)?", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the trailing ?
because this isn't phrased as a question
@@ -119,7 +119,7 @@ describe("rollback", () => { | |||
mockPostDeployment(); | |||
|
|||
mockPrompt({ | |||
text: "Please provide a message for this rollback (120 characters max, optional)?", | |||
text: "Please provide an optional message for this rollback (120 characters max)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that this test wasn't previously failing.
61d6424
to
a1d81c1
Compare
@@ -912,7 +916,10 @@ export async function validateDevServerSettings( | |||
// we want it to exit the Wrangler process early to allow the user to fix it. Calling it here forces | |||
// the error to be thrown where it will correctly exit the Wrangler process | |||
if (args.remote) { | |||
await getZoneIdForPreview(host, routes); | |||
const accountId = | |||
args.accountId ?? config.account_id ?? (await getAccountId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RamIdeas does this look right to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks about right. I just checked on the --x-dev-env
code path and it passes accountId to getWorkerAccountAndContext so this will continue to work when we flip that flag on by default
6066a3d
to
7836571
Compare
Fixes #4944 Trying. to fetch `/zones` fails when it spans more than 500 zones. The fix to use an account id when doing so. This patch passes the account id to the zones call, threading it through all the functions that require it.
7836571
to
cdcb94e
Compare
@@ -537,6 +537,9 @@ export async function startDev(args: StartDevOptions) { | |||
args.config || | |||
(args.script && findWranglerToml(path.dirname(args.script))); | |||
|
|||
const projectRoot = configPath && path.dirname(configPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved this up so I can reference config
at line 653
Fixes #4944
Trying to fetch
/zones
fails when it spans more than 500 zones. The fix to use an account id when doing so. This patch passes the account id to the zones call, threading it through all the functions that require it.Author has addressed the following