Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Allow running wrangler dev without an account_id #2030

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

jyn514
Copy link
Contributor

@jyn514 jyn514 commented Aug 16, 2021

Previously, running it with no account_id and no config in ~/.wrangler would give an error:

Error: config path does not exist /home/jnelson/.wrangler/config/default.toml. Try running `wrangler login` or `wrangler config`

This is not technically a regression from loading account_id lazily, because in wrangler 1.17 it would just error earlier that account_id was required:

🕵️  You can find your account_id in the right sidebar of your account's Workers page
Error: field `account_id` is required to deploy to workers.dev

However, it seems odd that you'd have to have an account to run an unauthenticated preview.
This changes wrangler to only require an account_id or config file if you're actually deploying your site.

Fixes #1525

@jyn514 jyn514 requested a review from a team as a code owner August 16, 2021 22:34
@jyn514 jyn514 force-pushed the jnelson/unauthenticated-dev branch from 6f84085 to 4488b0c Compare August 16, 2021 22:51
@jyn514
Copy link
Contributor Author

jyn514 commented Aug 16, 2021

So, I'm hitting more test failures ... the current ones are

thread 'settings::toml::tests::deployments::it_gets_deployments_with_route_and_workers_dev_true' panicked at 'assertion failed: manifest.get_deployments(environment).is_err()', src/settings/toml/tests/deployments.rs:508:5

thread 'settings::toml::tests::deployments::it_gets_deployments_with_routes_and_workers_dev_true' panicked at 'assertion failed: manifest.get_deployments(environment).is_err()', src/settings/toml/tests/deployments.rs:523:5

---- settings::toml::tests::deployments::it_errors_on_get_deployments_missing_account_id stdout ----
thread 'settings::toml::tests::deployments::it_errors_on_get_deployments_missing_account_id' panicked at 'assertion failed: manifest.get_deployments(environment).is_err()', src/settings/toml/tests/deployments.rs:99:5

I think this is because they expect get_deployments to fail eagerly, but now this only fails in deploy().

@jyn514
Copy link
Contributor Author

jyn514 commented Sep 1, 2021

@Electroid this is ready for review.

@jyn514
Copy link
Contributor Author

jyn514 commented Sep 1, 2021

@caass mentioned wanting to test this with Durable Objects and KV namespaces, but I'm not sure how to do that ... happy to do so if someone can write up instructions. I tried wrangler generate durable-objects https://github.com/cloudflare/durable-objects-template, but wrangler dev gives errors even without my changes:

Error: HTTP status client error (400 Bad Request) for url (https://api.cloudflare.com/client/v4/accounts/e1706d218241c1230155b4f91b3218af/workers/scripts/durable-objects/edge-preview)

@Electroid Electroid marked this pull request as ready for review September 1, 2021 18:05
Copy link
Contributor

@jspspike jspspike left a comment

Choose a reason for hiding this comment

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

Tested locally and seems to be working as intended, other than that one commented out tests it looks good to me

Previously, running it with no account_id and no config in ~/.wrangler would give an error:

```
Error: config path does not exist /home/jnelson/.wrangler/config/default.toml. Try running `wrangler login` or `wrangler config`
```

This is not technically a regression from loading account_id lazily, because in wrangler 1.17 it would just error earlier that account_id was required:
```
🕵️  You can find your account_id in the right sidebar of your account's Workers page
Error: field `account_id` is required to deploy to workers.dev
```

However, it seems odd that you'd have to have an account to run an unauthenticated preview.
This changes wrangler to only require an account_id or config file if you're actually deploying your site.

This also changes the tests not to expect that wrangler requires an account_id just to load the available deployments.
@jyn514 jyn514 merged commit 37caf3c into master Sep 23, 2021
@jyn514 jyn514 deleted the jnelson/unauthenticated-dev branch September 23, 2021 19:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: cannot start a local unauthenticated dev server without an account id in wrangler.toml
2 participants