Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
AlbertMarashi opened this issue Oct 19, 2023 · 4 comments · Fixed by #45
Closed

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

AlbertMarashi opened this issue Oct 19, 2023 · 4 comments · Fixed by #45
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@AlbertMarashi
Copy link
Collaborator

AlbertMarashi commented Oct 19, 2023

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 AlbertMarashi added enhancement New feature or request good first issue Good for newcomers labels Oct 19, 2023
@notdanilo
Copy link
Contributor

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

@rellfy
Copy link
Owner

rellfy commented Feb 19, 2024

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.

@greenboxal
Copy link

greenboxal commented Oct 22, 2024

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.

@InAnYan
Copy link

InAnYan commented Nov 14, 2024

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 😆

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants