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 queue binding type for workers scripts #1176

Merged
merged 2 commits into from
Jan 15, 2023

Conversation

jbw1991
Copy link
Contributor

@jbw1991 jbw1991 commented Jan 11, 2023

Description

Adds the Queue binding type. Workers scripts use this binding to write messages to a Queue.

Has your change been tested?

Types of changes

What sort of change does your code introduce/modify?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [x ] I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented (api.cloudflare.com or developers.cloudflare.com) and stable APIs.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 11, 2023

changelog detected ✅

@codecov-commenter
Copy link

Codecov Report

Merging #1176 (a3d8914) into master (6153c1e) will decrease coverage by 0.04%.
The diff coverage is 47.43%.

@@            Coverage Diff             @@
##           master    #1176      +/-   ##
==========================================
- Coverage   49.40%   49.35%   -0.05%     
==========================================
  Files         127      128       +1     
  Lines       12290    12432     +142     
==========================================
+ Hits         6072     6136      +64     
- Misses       4840     4892      +52     
- Partials     1378     1404      +26     
Impacted Files Coverage Δ
access_organization.go 64.70% <ø> (ø)
cloudflare_experimental.go 0.00% <0.00%> (ø)
utils.go 72.72% <ø> (ø)
cloudflare.go 68.37% <14.28%> (-0.34%) ⬇️
mtls_certificates.go 26.59% <26.59%> (ø)
workers_bindings.go 67.44% <40.00%> (-2.31%) ⬇️
origin_ca.go 57.26% <90.90%> (-2.09%) ⬇️
dns.go 72.97% <96.22%> (+9.24%) ⬆️
devices_managed_networks.go 41.55% <100.00%> (+9.09%) ⬆️
email_routing_destination.go 66.66% <100.00%> (+0.41%) ⬆️
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jbw1991
Copy link
Contributor Author

jbw1991 commented Jan 12, 2023

Changelog entry added!

As far as the API docs go, the workers script upload endpoint is documented (https://api.cloudflare.com/#worker-script-upload-worker) but it looks like it does not specifically cover each individual binding type.

I'm not sure if this repo contains documentation for the various workers binding types, but LMK if so and I can add a description.

// WorkerQueueBinding is a binding to a Workers Queue.
//
// https://developers.cloudflare.com/workers/platform/bindings/#queue-bindings
type WorkerQueueBinding struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

How are we differentiating between a producer and consumer binding? https://developers.cloudflare.com/queues/configuration/

Should we add an optional sub-resource to allow folks to configure the consumer? Something like https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloudfunctions_function#event_trigger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is only for the producer side of Queues config. I'll tackle all the consumer stuff separately, but yeah we are going to need to define a new type of sub-resource for consumers. Basically I was thinking that a script could have two sub resources: queue_bindings and queue_consumers (names could be changed...) that would handle each type.

Choose a reason for hiding this comment

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

@jbw1991 are you planning on tackling queue consumers anytime soon?

@jbw1991 jbw1991 changed the title WIP: Add queue binding type for workers scripts Add queue binding type for workers scripts Jan 13, 2023
@jacobbednarz jacobbednarz merged commit 300bc06 into cloudflare:master Jan 15, 2023
@github-actions github-actions bot added this to the v0.59.0 milestone Jan 15, 2023
github-actions bot pushed a commit that referenced this pull request Jan 15, 2023
@github-actions
Copy link
Contributor

This functionality has been released in v0.59.0.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

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.

5 participants