feat: Simplify Destinations

[bbernays]

Summary

This PR exposes a new method NewClient() that takes in a single struct. It returns a client that handles all of the read/write conversions for various file types

Here is how I expect destinations to use this functionality

// each destination will embed this client:
type Client struct {
	*filetypes.Client
}

// The spec will also embed the `Spec` struct from this package:
type Spec struct {
	Bucket   string `json:"bucket,omitempty"`
	Path     string `json:"path,omitempty"`
	NoRotate bool   `json:"no_rotate,omitempty"`
	*filetypes.FileSpec
}

func New(ctx context.Context, logger zerolog.Logger, spec specs.Destination) (destination.Client, error) {
	.
	.
	filetypesClient, err := filetypes.NewClient(c.pluginSpec.FileSpec)
	if err != nil {
		return nil, fmt.Errorf("failed to create filetypes client: %w", err)
	}
	c.Client = filetypesClient
	.
	.

The spec that users will use will look like this:

kind: destination
spec:
  name: "file"
  version: "v2.0.6"
  registry: "github"
  path: "cloudquery/file"
  write_mode: "overwrite-delete-stale"
  spec:
    format: "csv"
    format_spec:
      delimiter: ","

• Anything that defines the behavior of the data within the file should be a part of format_spec (csv headers, delimiters, replacing any sort of special characters)
• Anything that defines the behavior of the object or destination should be spec.spec (same level as format)

---

Use the following steps to ensure your PR is ready to be reviewed

• Read the contribution guidelines 🧑‍🎓
• Run go fmt to format your code 🖊
• Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
• Update or add tests 🧪
• Ensure the status checks below are successful ✅

[candiduslynx]

I feel that extracting client, transformer & reverse transformer into a separate struct that embeds the aforementioned elements might streamline the code a bit more

[disq]

Delimiter can be the NUL character, so depending on the zero value doesn't do it 100% :)

[hermanschaaf]

Seems like we're unlikely to run into this in practice, but if we want to be safe we could probably change Delimiter to string and use the first rune in the string (with validation that it doesn't have > 1 runes). Then we can check for the empty string here.

[yevgenypats]

I like that. One question - can users still use the old method or now they have to embed this client and it's all or nothing?

[bbernays]

I like that. One question - can users still use the old method or now they have to embed this client?

All of the old ways are still exposed... They should be used in cases where the default/ managed version is unsuitable

[yevgenypats]

Look good from quick look, will defer to @hermanschaaf re-review.

[hermanschaaf]

I think storing *csvSpec and *jsonSpec as state on the file spec is maybe a little bit unusual, but I can see why you went this route: so that you can call Validate() directly on the file spec, as long as UnmarshalSpec() was called first. This way we can add new file types to this library without updating the logic in all the destinations, which is quite a nice property. LGTM 👍

[candiduslynx]

thought:

1. make value receiver instead of pointer receiver for csvFile.Transformer{}
2. drop reverse transformer func from csvFile.Transformer{} as it should be inherited from embedded struct
3. you can change the field to be value here and not init it al all

[candiduslynx]

same for json, btw