This repository has been archived by the owner on Aug 3, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add credential checking logic when auth params are provided in wrangler config #842
Add credential checking logic when auth params are provided in wrangler config #842
Changes from 2 commits
f1a721c
39bda96
60150c1
d7d627d
ec9ae69
2b4b5f6
655b1c0
bb60581
b8ac9f7
5fbafb5
5d453f6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
should we link here (and above) to the docs about finding your credentials? i wouldn't link anywhere in the dashboard because we don't control those, but the docs should be pretty consistent. open to being wrong about this, not blocking.
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.
Weirdly enough, the best docs are in the help center (there's not a comprehensive set of API token docs in the v4 API documentation yet...). I can add a link to this.
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.
we should have something in the workers wrangler quick start. if it doesn't already exist, we should update it to include api token instructions as part of this project
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.
https://developers.cloudflare.com/workers/quickstart/#global-api-key
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 think the above link is the most stable atm
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.
The link above seems to have outdated documentation, I'm going to open up a workers docs ticket to fix that (as well as add a section for api tokens). I'll omit the link from Wrangler for now, but will add them when the docs are ready!
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 think it is fine to include the link, given the location will not change, only the content, and the content must be updated before we release. one less loose thread to track.
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 game to change the link to be a general https://developers.cloudflare.com/workers/quickstart/#authentication link (note: this endpoint currently doesn't exist). This link would contain information about both API tokens and API keys... what do you think? In the meantime I'll add the existing link.
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.
ah that seems like the way to go. yeah add the existing link, write an issue for us here, and an accompanying issue for docs (cc @victoriabernard92 ) and this is good to merge.