cloudflare-go icon indicating copy to clipboard operation
cloudflare-go copied to clipboard

Ability to round-trip worker bindings

Open sodabrew opened this issue 3 years ago • 1 comments

Current cloudflare-go version

0.45

Description

I am writing a utility to download a script, modify it, and upload it again to a new name.

It would be useful if the input and output formats for the Bindings structure matched up. I am reading the code to see if there's a simple enough way to transform the structures, but I figured I'd file this before I got too deep.

Use cases

This is similar to the Workers Environments use case, so future work on that product feature might subsume this eventually, but download-modify-upload is still a valuable capability.

Potential cloudflare-go usage

cfapi, err := cloudflare.New(config.APIKey, config.APIEmail, cloudflare.UsingAccount(config.AccountID))
script, err := cfapi.DownloadWorker(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
bindings, err := cfapi.ListWorkerBindings(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
response, err := cfapi.UploadWorkerWithBindings(ctx,
  &cloudflare.WorkerRequestParams{
    ScriptName: "output",
  },
  &cloudflare.WorkerScriptParams{
    Script: script.Script,
    Bindings: bindings.BindingList,
  },
)

Unfortunately, this won't compile:

cannot use bindings.BindingList (variable of type []cloudflare.WorkerBindingListItem)
  as type map[string]cloudflare.WorkerBinding in struct literal

References

Aha, this might do it. I feel like this comment is a clue that maybe the data structure mismatch ought to have been resolved in the code review process. How would you feel if I put up a PR to make UploadWorkerWithBindings accept an array instead of a map? Was the idea to use a map to avoid the case of two bindings with the same name? In that case, should the ListWorkerBindings return an array? Maybe a convenience method to munge between them?

https://github.com/cloudflare/cloudflare-go/blob/9a9f293157bf887f5e207fc9a3535401c3ce7f62/workers_test.go#L233-L240

sodabrew avatar Jul 28 '22 05:07 sodabrew

Got it. Right, so this feels totally unnecessary impedance mismatch. Guidance on a PR to not need this code would be greatly appreciated!

bindingMap:= make(map[string]cloudflare.WorkerBinding)
for _, binding := range bindings.BindingList {
        bindingMap[binding.Name] = binding.Binding
}

End-to-end working example:

cfapi, err := cloudflare.New(config.APIKey, config.APIEmail, cloudflare.UsingAccount(config.AccountID))
script, err := cfapi.DownloadWorker(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
bindings, err := cfapi.ListWorkerBindings(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })

bindingMap:= make(map[string]cloudflare.WorkerBinding)
for _, binding := range bindings.BindingList {
        bindingMap[binding.Name] = binding.Binding
}

response, err := cfapi.UploadWorkerWithBindings(ctx,
  &cloudflare.WorkerRequestParams{
    ScriptName: "output",
  },
  &cloudflare.WorkerScriptParams{
    Script: script.Script,
    Bindings: bindingMap,
  },
)

sodabrew avatar Jul 28 '22 05:07 sodabrew