-
Notifications
You must be signed in to change notification settings - Fork 1k
Add Cloudflare Pipelines commands #6629
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
Conversation
🦋 Changeset detectedLatest commit: ea586a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
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/10728139021/npm-package-wrangler-6629You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6629/npm-package-wrangler-6629Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-wrangler-6629 dev path/to/script.jsAdditional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-create-cloudflare-6629 --no-auto-updatenpm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-cloudflare-kv-asset-handler-6629npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-miniflare-6629npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-cloudflare-pages-shared-6629npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-cloudflare-vitest-pool-workers-6629npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-cloudflare-workers-editor-shared-6629npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10728139021/npm-package-cloudflare-workers-shared-6629Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
d41f5f7 to
944728f
Compare
|
@oliy I'm seeing a fair amount of linting errors here. Can you run ESLint locally and push fixes? |
| const secret_access_key = sha256(serviceToken.value); | ||
|
|
||
| // wait for token to settle/propagate | ||
| await sleep(3000); |
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.
Instead of waiting here, let's find another way to assert that the token has propagated. Can we poll an endpoint to confirm?
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 agree this isn't ideal, but the alternative seemed rougher. The confirmation would require the use of the S3 API, which would be either:
- much more complication, since we'd have to generate a signature ourselves: https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html
- include a large package that implements the s3 standard, just to get endpoint verification.
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.
To address the delay in tests, I've added a delay bypass for tests.
| } | ||
|
|
||
| logger.log(`🌀 Creating pipeline named "${name}"`); | ||
| const pipeline = await createPipeline(accountId, pipelineConfig); |
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.
What should happen if this fails?
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.
It throws an exception with the appropriate error message. Is there a better mechanism for errors? This seemed to be the common pattern among other handlers.
|
Closing because this was superseded by this PR: #6649 |
What this PR solves / how to test
Adds new Cloudflare Pipelines commands: PIPE-27, PIPE-96, PIPE-110
Author has addressed the following