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

Add DeliveryDelay and RetryDelay to create Queue and Add Worker Consumer commands. #5102

Merged
merged 17 commits into from
Mar 26, 2024

Conversation

pmiguel
Copy link
Contributor

@pmiguel pmiguel commented Feb 26, 2024

Fixes # MQ-499 internally

CR for merge under LDW: # CR-850147

What this PR solves / how to test:

Adds support for message delivery delay on create new Queue, and retry delay when create new consumer.

Author has addressed the following:

Copy link

changeset-bot bot commented Feb 26, 2024

🦋 Changeset detected

Latest commit: fa222f2

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

Copy link
Contributor

github-actions bot commented Feb 26, 2024

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/8378697493/npm-package-wrangler-5102

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

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

Or you can use npx with this latest build directly:

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

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


wrangler@3.36.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20240314.0
workerd 1.20240320.1 1.20240320.1
workerd --version 1.20240320.1 2024-03-20

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

Copy link
Contributor

@jbwcloudflare jbwcloudflare left a comment

Choose a reason for hiding this comment

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

Looking good, I think there are some test failures that need to be fixed though.

packages/wrangler/src/queues/cli/commands/create.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/create.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/create.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/consumer/add.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/consumer/add.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/queues.test.ts Outdated Show resolved Hide resolved
@pmiguel pmiguel force-pushed the pleal/queue-delays branch 4 times, most recently from 68b9d1b to 7a71f7e Compare February 28, 2024 15:01
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 91.17647% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 72.27%. Comparing base (e88ad44) to head (fa222f2).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5102      +/-   ##
==========================================
+ Coverage   72.15%   72.27%   +0.11%     
==========================================
  Files         317      318       +1     
  Lines       16369    16398      +29     
  Branches     4168     4174       +6     
==========================================
+ Hits        11811    11851      +40     
+ Misses       4558     4547      -11     
Files Coverage Δ
...ler/src/queues/cli/commands/consumer/worker/add.ts 100.00% <100.00%> (ø)
...ackages/wrangler/src/queues/cli/commands/create.ts 100.00% <100.00%> (ø)
packages/wrangler/src/queues/client.ts 96.42% <ø> (ø)
packages/wrangler/src/queues/constants.ts 100.00% <100.00%> (ø)
.../src/queues/cli/commands/consumer/http-pull/add.ts 94.11% <66.66%> (-5.89%) ⬇️
packages/wrangler/src/queues/utils.ts 88.23% <77.77%> (-0.66%) ⬇️

... and 7 files with indirect coverage changes

@pmiguel pmiguel force-pushed the pleal/queue-delays branch 2 times, most recently from 22cc5a5 to 17dd56d Compare February 28, 2024 17:50
@jbwcloudflare
Copy link
Contributor

This looks solid to me, I think you are good to open it up for review by the wrangler team.

@pmiguel pmiguel changed the title [WIP] Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Feb 29, 2024
@pmiguel pmiguel marked this pull request as ready for review February 29, 2024 17:38
@pmiguel pmiguel requested a review from a team as a code owner February 29, 2024 17:38
.changeset/warm-seahorses-trade.md Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/queues.test.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/consumer/add.ts Outdated Show resolved Hide resolved
packages/wrangler/src/queues/cli/commands/create.ts Outdated Show resolved Hide resolved
@pmiguel pmiguel requested a review from mrbbot March 8, 2024 09:50
@petebacondarwin
Copy link
Contributor

What does this mean?

Associated docs
 Not necessary because: full range of options not yet documented.

Are you saying that you don't need to add docs for this because there are none at all yet?
At the least I think it would be good to create a placeholder issue in the docs repo, to track that this needs to be done at some point (soonish?).

@pmiguel
Copy link
Contributor Author

pmiguel commented Mar 8, 2024

@petebacondarwin

What does this mean?

Associated docs
 Not necessary because: full range of options not yet documented.

Are you saying that you don't need to add docs for this because there are none at all yet? At the least I think it would be good to create a placeholder issue in the docs repo, to track that this needs to be done at some point (soonish?).

There is documentation (https://github.com/cloudflare/cloudflare-docs/blob/production/content/workers/wrangler/commands.md#queues), it's just the [OPTIONS] that are lacking. I'll check if we already have an issue for that.

@pmiguel pmiguel changed the title Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. [do not merge] Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Mar 8, 2024
@petebacondarwin
Copy link
Contributor

I've created an issue in the docs repo to track the addition of the OPTIONS descriptions: cloudflare/cloudflare-docs#13370.

I see you have changed the title to [do not merge]. So I will also change this to "draft". Please mark it as "ready to review" when you are ready.

@petebacondarwin petebacondarwin marked this pull request as draft March 11, 2024 21:08
@pmiguel pmiguel force-pushed the pleal/queue-delays branch 3 times, most recently from cba3904 to b3399a1 Compare March 21, 2024 16:24
@pmiguel pmiguel changed the title [do not merge] Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Mar 21, 2024
@pmiguel pmiguel marked this pull request as ready for review March 21, 2024 16:29
@pmiguel pmiguel changed the title Add DeliveryDelay and RetryDelay to create Queue/Consumer commands. Add DeliveryDelay and RetryDelay to create Queue and Add Worker Consumer commands. Mar 21, 2024
@pmiguel
Copy link
Contributor Author

pmiguel commented Mar 26, 2024

Approved under LDW with CR-850147

@mrbbot mrbbot merged commit ba52208 into cloudflare:main Mar 26, 2024
15 checks passed
@mrbbot mrbbot added the ldw-exempt PRs marked with this label can be included in releases during LDW. label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ldw-exempt PRs marked with this label can be included in releases during LDW.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants