Create PineconeClientConfig struct

[ericapywang]

Problem

Currently, PineconeClient::new() takes in four optional arguments, which are often not provided. Calling PineconeClient::new(None, None, None, None) isn't the best user experience.

Solution

I created a PineconeClientConfig struct, which takes in all arguments that PineconeClient::new() would take in, and then pass in just this one struct into a with_config function. This struct also implements Default, so the user can set the parameters they want, and use ..Default::default() to fill in the rest. The new function now doesn't take in any parameters and will call with_config with all parameters set to None.

Type of Change

• 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 not work as expected)
• This change requires a documentation update
• Infrastructure change (CI configs, etc)
• Non-code change (docs, etc)
• None of the above: (explain here)

Test Plan

Test cases should pass.

[ssmith-pc]

This still doesn't really spark joy for me:

PineconeClient::new(Default::default())

What about having a default no-param version:

PineconeClient::new()

and some form of overload:

PineconeClient::from_config(PineconeClientConfig { .. })
or
PineconeClient::with_config(PineconeClientConfig { .. })
or
PineconeClient::new_with_config(PineconeClientConfig { .. })

[ssmith-pc]

Looks good to me, but as this is a subjective change I think we should get multiple opinions

[austin-denoble]

Really nice work, I like this approach a lot. It's very similar to what we've done in a number of other SDKs, and feels a lot cleaner to read and work with. 🎉

[haruska]

This is indeed nicer than the new(None, None, ...) pattern before. It seems most similar to a Golang-style of an input struct to denote named parameters.

An alternative for consideration:

• Implement the Default trait for the PineconeClient struct. If the caller wants the old new(None, ...) behavior, they can just call PineconeClient::default() instead.
• Implement pub fn client() -> Result<PineconeClient, Error> on PineconeClientConfig to generate a client from the config.
• Implement the TryFrom trait for PineconeClient that calls the client() function
• Remove fn PineconeClient::new

[haruska]

Nice improvement!

[haruska]

can we change to &self or does the call need to own/consume the config?

[ericapywang]

I think the call needs to consume the config, or it would need to clone all the fields

[austin-denoble]

LGTM, just one README nit. Great work!

[austin-denoble]

nit: Might be worth mentioning that configuration values will be read from the environment in this case.