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.

Sign up for GitHub

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

Jump to bottom

Mention env var when using API Token and whoami/log(out|in) commands #4925

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Mention env var when using API Token and whoami/log(out|in) commands #4925

merged 3 commits into from
Jul 10, 2024

Conversation

dom96
Copy link
Contributor

@dom96 dom96 commented Feb 5, 2024 •
edited
Loading

I ran into this while trying to log in to my other account. Didn't realise wrangler uses this env var, so was confused why I couldn't logout.

What this PR solves / how to test:

$ pnpm run build --filter wrangler
$ pnpm run --filter wrangler start whoami
$ pnpm run --filter wrangler start logout
$ pnpm run --filter wrangler start login

Screenshot 2024-02-05 at 18 47 08
Screenshot 2024-02-05 at 18 47 47
Screenshot 2024-02-05 at 18 56 26

Author has addressed the following:

  • Tests
    • Included
    • Not necessary because: simple change
  • Changeset (Changeset guidelines)
    • Included
    • Not necessary because:
  • Associated docs
    • Issue(s)/PR(s):
    • Not necessary because: simple change

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: a4420a3

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

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers 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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 5, 2024 •
edited
Loading

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/9876830267/npm-package-wrangler-4925

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

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/4925/npm-package-wrangler-4925

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-wrangler-4925 dev path/to/script.js
Additional artifacts: 
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-create-cloudflare-4925 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-cloudflare-kv-asset-handler-4925
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-miniflare-4925
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-cloudflare-pages-shared-4925
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9876830267/npm-package-cloudflare-vitest-pool-workers-4925

Note that these links will no longer work once the GitHub Actions artifact expires.

[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20240701.0
workerd 1.20240701.0 1.20240701.0
workerd --version 1.20240701.0 2024-07-01

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@dom96 dom96 force-pushed the dominik/better-errors-when-using-api-token branch from 99c9ba5 to 86af1b5 Compare February 5, 2024 18:56
@dom96 dom96 changed the title Mention env var when using API Token and whoami/logout commands Mention env var when using API Token and whoami/log(out|in) commands Feb 5, 2024
@dom96 dom96 force-pushed the dominik/better-errors-when-using-api-token branch from 86af1b5 to 66285aa Compare February 5, 2024 18:57
@dom96 dom96 marked this pull request as ready for review February 5, 2024 18:58
@dom96 dom96 requested a review from a team as a code owner February 5, 2024 18:58
@petebacondarwin
Copy link
Contributor

Hey @dom96 - thanks for improving this DX.

  • The code is failing a formatting check. Can you try running pnpm fix in the root of the repo to fix this?
  • The code is failing unit tests - a snapshot need updating. Can you try running pnpm -F wrangler test:ci -u

@dom96 dom96 force-pushed the dominik/better-errors-when-using-api-token branch from 66285aa to 72f9e43 Compare February 16, 2024 16:09
@dom96
Copy link
Contributor Author

dom96 commented Feb 16, 2024

@petebacondarwin thanks for the tips! I updated the snapshots whose updates seemed to make sense, but I did get a bunch of other updated snapshots that looked wrong (I also saw many test failures, from previous PRs it didn't seem to be to do with my changes, so I'm unsure whether I should dive deeper yet)

@petebacondarwin
Copy link
Contributor

Let's see how the CI pans out. I can look into any random failures.

@dom96 dom96 force-pushed the dominik/better-errors-when-using-api-token branch from 72f9e43 to b4f81e3 Compare March 4, 2024 10:32
@petebacondarwin petebacondarwin self-assigned this Jul 10, 2024
@petebacondarwin petebacondarwin force-pushed the dominik/better-errors-when-using-api-token branch 2 times, most recently from 989cc77 to 0619717 Compare July 10, 2024 11:42
petebacondarwin
petebacondarwin reviewed Jul 10, 2024
View reviewed changes
packages/wrangler/src/__tests__/whoami.test.tsx Outdated

"
`);
expect(std.warn).toMatchInlineSnapshot(`""`);
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs fixing...

@petebacondarwin petebacondarwin force-pushed the dominik/better-errors-when-using-api-token branch from 0619717 to e5f6831 Compare July 10, 2024 12:51
dom96 and others added 3 commits July 10, 2024 13:55
@dom96 @petebacondarwin
Mention env var when using API Token and whoami/log(out|in) comma…
57664a7 
…nds.
@petebacondarwin
fixup tests
6c26770
@petebacondarwin
disable nodejs-hybrid-app fixture test
a4420a3 
This test hits a public database that has started
failing with

> too many connections for role 'reader'

We need to find an alternative (ideally local) DB to test against.
@petebacondarwin petebacondarwin force-pushed the dominik/better-errors-when-using-api-token branch from e5f6831 to a4420a3 Compare July 10, 2024 15:11
@petebacondarwin petebacondarwin merged commit 7d4a4d0 into cloudflare:main Jul 10, 2024
18 checks passed
@workers-devprod workers-devprod mentioned this pull request Jul 10, 2024
Merged
@dom96
Copy link
Contributor Author

dom96 commented Jul 11, 2024

Thank you for the help in getting this merged in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petebacondarwin petebacondarwin petebacondarwin approved these changes

Assignees

@petebacondarwin petebacondarwin

Labels
None yet
Projects
workers-sdk
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dom96 @petebacondarwin