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

Stop racing secret uploads #255

Merged

Conversation

matthewdavidrodgers
Copy link
Contributor

For up to date versions of wrangler, secrets are uploaded via the 'secret:bulk' command, which batches updates in a single API call.

For versions of wrangler without that capability, the action falls back to the single 'secret put' command for each secret. It races all these with a Promise.all()

Unfortunately, the single secret API cannot handle concurrency - at best, these calls have to wait on one another, holding requests open all the while. Often it times out and errors.

This fixes the legacy secret upload errors by making these calls serially instead of concurrently.

For up to date versions of wrangler, secrets are uploaded via the
'secret:bulk' command, which batches updates in a single API call.

For versions of wrangler without that capability, the action falls back
to the single 'secret put' command for each secret. It races all these
with a Promise.all()

Unfortunately, the single secret API cannot handle concurrency - at
best, these calls have to wait on one another, holding requests open
all the while. Often it times out and errors.

This fixes the legacy secret upload errors by making these calls
serially instead of concurrently.
@matthewdavidrodgers matthewdavidrodgers requested a review from a team as a code owner April 26, 2024 23:02
@RamIdeas RamIdeas mentioned this pull request Apr 30, 2024
@RamIdeas
Copy link
Contributor

I think this change makes sense. If you're on a legacy version of wrangler, pre-secret:bulk, you'll get a slower but more stable experience. You'll need to upgrade to newer versions of wrangler, post-secret:bulk, to get the faster bulk upload.

Tests were run in #256, since this PR came from a fork.

@RamIdeas RamIdeas merged commit b05934f into cloudflare:main Apr 30, 2024
2 of 3 checks passed
@github-actions github-actions bot mentioned this pull request Apr 30, 2024
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.

4 participants