fix: don't attempt to login during a --dryRun #1037
Conversation
🦋 Changeset detectedLatest commit: 70ee2ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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.developers.workers.dev/runs/2338904953/npm-package-wrangler-1037You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1037/npm-package-wrangler-1037Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2338904953/npm-package-wrangler-1037 dev path/to/script.js |
| @@ -1256,7 +1256,7 @@ export async function main(argv: string[]): Promise<void> { | |||
| ); | |||
| } | |||
|
|
|||
| const accountId = await requireAuth(config); | |||
There was a problem hiding this comment.
@threepointone Is it an issue that during --dry-run, accountId won't be set even if you're logged in? We're not like generating a manifest with accountIds/paths that rely on accountId being set?
There was a problem hiding this comment.
Well we want zero requests during dry run, so maybe it's good that we enforce it doubly here. We'll want to generate a manifest, but we probably won't include account id in it (yet).
rozenmd
force-pushed
the
dont-login-during-dryrun
branch
from
989a95e
to
1cb5db6
Compare
|
FWIW I think there is potential for there to be two kinds of "dry-run". One is what we are implementing here, which is where we promise never to make any network requests at all. The other is where we promise not to do any network requests that are not readonly. The latter could be useful since we could literally "dry-run" every aspect of the logic, including doing verifications that require API interactions, such as listing off which assets would need uploading. |
* fix: don't attempt to login during a --dryRun * Create wild-seas-breathe.md * chore: update test to explicitly set CLOUDFLARE_ACCOUNT_ID to an empty string
Closes #1023
//TODO: