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

WC-1473: Add support for script content and script settings read/update endpoints #1361

Merged
merged 6 commits into from
Aug 22, 2023

Conversation

echen67
Copy link

@echen67 echen67 commented Aug 7, 2023

This PR is to support new endpoints that allow the user to read/update either only the script content or only the script settings of a worker, as opposed to currently having to always update everything together. This makes it easier on the user when they only need to deal with one aspect of their worker.

Has your change been tested?

I have added new tests to cover this change.

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.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented in cloudflare/api-schemas
    and relies on stable APIs.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

changelog detected ✅

workers.go Outdated Show resolved Hide resolved
workers.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #1361 (1865065) into master (b9ac804) will increase coverage by 0.08%.
Report is 111 commits behind head on master.
The diff coverage is 53.98%.

@@            Coverage Diff             @@
##           master    #1361      +/-   ##
==========================================
+ Coverage   48.33%   48.42%   +0.08%     
==========================================
  Files         133      135       +2     
  Lines       13023    13303     +280     
==========================================
+ Hits         6295     6442     +147     
- Misses       5201     5291      +90     
- Partials     1527     1570      +43     
Files Changed Coverage Δ
device_posture_rule.go 61.80% <ø> (ø)
load_balancing.go 59.40% <ø> (ø)
teams_accounts.go 53.84% <ø> (ø)
workers_account_settings.go 29.41% <ø> (ø)
workers_cron_triggers.go 29.41% <ø> (ø)
workers_domain.go 67.18% <ø> (ø)
workers_kv.go 39.68% <ø> (ø)
workers_routes.go 31.25% <ø> (ø)
workers_subdomain.go 57.14% <ø> (ø)
workers_tail.go 52.00% <ø> (ø)
... and 10 more

... and 106 files with indirect coverage changes

workers.go Outdated Show resolved Hide resolved
workers.go Outdated Show resolved Hide resolved
@jacobbednarz
Copy link
Member

looks good in general, just a couple of notes from https://github.com/cloudflare/cloudflare-go/blob/master/docs/conventions.md#methods that we should update here for consistency.

@jacobbednarz
Copy link
Member

thanks @echen67, this looks awesome. once we have those API schemas land, we're good to go here.

return "", ErrMissingAccountID
}

uri := fmt.Sprintf("/accounts/%s/workers/scripts/%s/content/v2", rc.Identifier, scriptName)
Copy link
Member

Choose a reason for hiding this comment

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

the API docs suggest this is just /accounts/:account_id/workers/scripts/:script/content. is that correct?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, technically both work, but /v2 is preferred for this GET endpoint so I'll update the API docs to match. Thanks for catching this!

@jacobbednarz jacobbednarz merged commit 3b0a9e3 into cloudflare:master Aug 22, 2023
11 checks passed
@github-actions github-actions bot added this to the v0.76.0 milestone Aug 22, 2023
@jacobbednarz
Copy link
Member

thanks for getting this one over the line @echen67 🚀

github-actions bot pushed a commit that referenced this pull request Aug 22, 2023
@github-actions
Copy link
Contributor

This functionality has been released in v0.76.0.

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants