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
Add support for Cloudflare Workers #5258
Conversation
added headers in enterprise template
@pinkerton |
@horike37 That was accidental. I must have committed it by mistake when rebasing / merging. Will revert. |
Un-committed the accidental README changes. |
service: | ||
name: hello-world | ||
config: | ||
accountId: CLOUDFLARE_ACCOUNT_ID |
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.
@pinkerton Is the accountId
really required here. I've tried to deploy and remove without it and it seems to work fine without it.
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.
Interesting, that may have been left in accidentally. I'm double-checking if we can remove that.
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.
So, if the user has the multiscript
feature enabled, the api call uses accountId
as one of the parameters.
https://github.com/cloudflare/serverless-cloudflare-workers/blob/master/deploy/lib/multiscript.js#L27
Whereas, for singleScript
feature, accountId
is not required.
https://github.com/cloudflare/serverless-cloudflare-workers/blob/master/deploy/lib/singlescript.js#L27
All in all, accountId
is required in MultiScript, but optional in SingleScript
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.
Tested end to end. Missing documentation added. LGTM
Thanks so much for taking a look @RaeesBhatti. What version of the CLI tool will this ship in? It looks like maybe 1.30.4 or 1.31.0. Our docs have an old version in them as a placeholder for the minimum version that I'd like to update. I also need to add our images to our documentation. And how do we get our docs linked on this page https://serverless.com/framework/docs/ ? |
@pinkerton The docs on the website will be updated automatically on new build. But I noticed that |
Thanks @RaeesBhatti. I'm working on publishing our plugin to NPM now. Running into an issue where I can publish to our org as an org-scoped package, but that would make the name Will have this resolved soon. |
@pinkerton it'll likely be |
Made some copy and visual changes to the docs. They should be good to go! |
Thanks for catching that @RaeesBhatti |
What did you implement:
Closes #4948
This PR adds templates and documentation for Cloudflare Workers as a part of the provider plugin that we're reviewing and will publish on NPM very soon.
How did you implement it:
Added
cloudflare-workers
andcloudflare-workers-enterprise
templates and whitelisted them.Regarding documentation, I am not 100% sure how this is generated and how images will be hosted (I see they are mostly in an S3 bucket).
How can we verify it:
export CLOUDFLARE_AUTH_KEY=YOUR_CLOUDFLARE_GLOBAL_API_KEY
(get your Global API key from your Cloudflare profile page).export CLOUDFLARE_AUTH_EMAIL=you@example.com
git clone https://github.com/pinkerton/serverless.git
git clone https://github.com/cloudflare/serverless-cloudflare-workers.git
cd serverless
npm install
npm link ../serverless-cloudflare-workers
cd ..
./serverless/bin/serverless create --template cloudflare-workers --path my-project
cd my-project
npm install
npm link ../serverless-cloudflare-workers
serverless.yml
(get it from the URL when using the Cloudflare dashboard).serverless.yml
(zoneId
from theoverview
tab after selecting the desired zone from the Cloudflare dashboard).serverless.yml
.serverless deploy
serverless invoke -f helloWorld
serverless create
serverless deploy
&serverless invoke
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO