search

libninjacom / plaid-rs

Rust client library for Plaid, generated from Plaid's OpenAPI spec
https://docs.rs/plaid/
MIT License
11 stars 6 forks source link

PLAID_ENV doesn't match the plaid quickstart #11

Closed benwr closed 10 months ago

benwr commented 1 year ago

Hi - it's possible that I've misunderstood something about this crate, but my current understanding is that PLAID_ENV needs to be a URL (or else you get an inscrutable unwrap error from deep inside HttpClient). This is fine, but doesn't match the plaid quickstart (in which the PLAID_ENV variable should be set to one of sandbox, development, or production).

If I've understood this situation correctly, I think it would make sense to do one of the following:

  1. Read from a different environment variable instead to make it clear that they're different (e.g. PLAID_BASE_URI)
  2. Switch the semantics to match the quickstart
  3. Explain in the documentation that PLAID_ENV should be a base URI, and give an example
  4. Dynamically choose between the current semantics and the quickstart semantics

1 and 2 would be breaking changes; probably 3 is easiest. In some extremely strict sense, 4 is also a breaking change, but I think in practice it would only prevent runtime crashes (since sandbox, development, and production aren't valid base URIs)

kurtbuilds commented 1 year ago

Yeah, thanks for catching that. Care to create a PR to update the readme?

mentheosis commented 10 months ago

heres a PR to explain the var better in the readme: https://github.com/libninjacom/plaid-rs/pull/14/files

I was just stuck on this same thing for a bit, happy to make docs better :)

kurtbuilds commented 10 months ago

Thanks. It'd be good to update the library to be more consistent, I just haven't had the time.

kurtbuilds commented 10 months ago

With the latest version of the library (8.x), this is fixed. PLAID_ENV takes one of sandbox, development, and production.