openai icon indicating copy to clipboard operation
openai copied to clipboard

We should probably use RwLock for API key & base url?

Open AlbertMarashi opened this issue 2 years ago • 2 comments

Problem

We're currently using Mutexes but it makes sense to use a RwLock since it allows multiple readers at the same time.

Solution

Use RwLock

Let me know your thoughts

AlbertMarashi avatar Oct 19 '23 00:10 AlbertMarashi

TBH, I would rather have a structure holding the key instead of setting it globally.

notdanilo avatar Feb 19 '24 04:02 notdanilo

TBH, I would rather have a structure holding the key instead of setting it globally.

We could keep the option of setting a global key, for simplicity, and also allow for specifying a key (to override the global) within the builder pattern for each API. This keeps it simple for users that only need to use a single key while allowing for users that need multiple keys.

rellfy avatar Feb 19 '24 05:02 rellfy

TBH, I would rather have a structure holding the key instead of setting it globally.

Same. To be honest, I don't expect a library like this to even assume any defaults from the env.

greenboxal avatar Oct 22 '24 15:10 greenboxal

IMHO

  • It's not idiomatic Rust.
  • It's not hard to program a local struct.
  • Tons of libraries provide a Client struct.
  • Why would you make global variables at all in 2024 (or in 2022, when you made a first commit), considering how they are perceived now?

Sorry, if I sound too critical. But I just got highly triggered :laughing:

You have a very good and interesting library, but I would certainly NOT use it in my projects.

What if I want to build a multi-agent system? Use different, but OpenAI compatible API, different keys for different projects?, etc.

InAnYan avatar Nov 14 '24 07:11 InAnYan