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

fix wrangler publish --dry-run to not require authentication with queues #2631

Merged
merged 1 commit into from
Jan 27, 2023
Merged

fix wrangler publish --dry-run to not require authentication with queues #2631

merged 1 commit into from
Jan 27, 2023

Conversation

thibmeu
Copy link
Contributor

@thibmeu thibmeu commented Jan 27, 2023

When wrangler configuration has queues, wrangler publish dry-run requires the user to be logged in.

This is not the expected behavior.

This originates from wrangler publish ensuring that configured queues actually exists on the user account.
This should not be a concern for dry-run. This commit fixes this.

What this PR solves / how to test:

Associated docs issues/PR:

Author has included the following, where applicable:

- [ ] Tests

  • Changeset

Reviewer has performed the following, where applicable:

- [ ] Checked for inclusion of relevant tests

  • Checked for inclusion of a relevant changeset
    - [ ] Checked for creation of associated docs updates
  • Manually pulled down the changes and spot-tested

Fixes #2630 .

@thibmeu thibmeu requested a review from a team as a code owner January 27, 2023 13:04
@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2023

🦋 Changeset detected

Latest commit: 5a9d2d6

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

This PR includes changesets to release 1 package
Name Type
wrangler 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
Copy link
Contributor

github-actions bot commented Jan 27, 2023

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/runs/4025332909/npm-package-wrangler-2631

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/runs/4025332909/npm-package-wrangler-2631 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/runs/4025332909/npm-package-cloudflare-pages-shared-2631

Copy link
Contributor

@rozenmd rozenmd left a comment

Choose a reason for hiding this comment

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

Can you add a changeset? npx changeset in the repo root

@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Merging #2631 (5a9d2d6) into main (04d8a31) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2631      +/-   ##
==========================================
- Coverage   73.32%   73.32%   -0.01%     
==========================================
  Files         159      159              
  Lines        9852     9848       -4     
  Branches     2627     2626       -1     
==========================================
- Hits         7224     7221       -3     
+ Misses       2628     2627       -1     
Impacted Files Coverage Δ
packages/wrangler/src/publish/publish.ts 86.70% <100.00%> (ø)
packages/wrangler/src/dev.tsx 84.61% <0.00%> (-0.43%) ⬇️
packages/wrangler/src/index.ts 83.46% <0.00%> (-0.27%) ⬇️
packages/wrangler/src/git-client.ts 79.16% <0.00%> (+2.08%) ⬆️
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts 100.00% <0.00%> (+5.26%) ⬆️

When wrangler configuration has queues, wrangler publish dry-run requires
the user to be logged in.

This is not the expected behavior.

This originates from wrangler publish ensuring that configured queues
actually exists on the user account.
This should not be a concern for dry-run. This commit fixes this.
@thibmeu
Copy link
Contributor Author

thibmeu commented Jan 27, 2023

Can you add a changeset? npx changeset in the repo root

done

Copy link
Contributor

@rozenmd rozenmd left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thibmeu
Copy link
Contributor Author

thibmeu commented Jan 27, 2023

thanks @rozenmd. Anything special to merge the change? It seems I don't have permission to do this.

@rozenmd
Copy link
Contributor

rozenmd commented Jan 27, 2023

Oh sorry about that, merging

@rozenmd rozenmd merged commit 6b3fe5e into cloudflare:main Jan 27, 2023
@github-actions github-actions bot mentioned this pull request Jan 27, 2023
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: wrangler publish --dry-run should not require authentication
2 participants