Executor returned by llm_chain_openai::chatgpt::Executor::for_client() does not implement llm_chain::traits::Executor ([E0277])

[AnthonyMichaelTDM]

So, in my use case, I need an openai executor that uses an organization-id read from an environment variable, looking at the source code for the llm_chain_openai crate (as the documentation is kinda lacking), it would seem that the llm_chain_openai::chatgpt::Executor::for_client() would let me do that by letting me specify the async_openai client to use.

However, while this works

use async_openai::Client;
use lazy_static::lazy_static;
use llm_chain_openai::chatgpt::{Executor, Model, PerInvocation};

lazy_static! {
    pub static ref OPENAI_EXECUTOR: Executor = Executor::for_client(
        {
            let org_id = std::env::var("OPENAI_ORG_ID").unwrap_or_else(|_| "".to_string());
            if org_id.is_empty() {
                Client::new()
            } else {
                Client::new().with_org_id(org_id)
            }
        },
        Some(PerInvocation::new().for_model(Model::ChatGPT3_5Turbo)),
    );
}

it doesn't give me an Executor that implements the llm_chain::traits::Executor trait, meaning I can't actually use it for anything useful

is this intended behavior? if so, why? and if not, what can I/we do about it?

here is the specific error: rustc: the trait bound `OPENAI_EXECUTOR: llm_chain::traits::Executor` is not satisfied the trait `llm_chain::traits::Executor` is implemented for `llm_chain_openai::chatgpt::Executor` [E0277]

[AnthonyMichaelTDM]

in terms of potential solutions, I think the easiest is to either:

• add organization IDs as a supported PerExecutor option,
• add async_openai clients as a supported option,
• add a function that modifies the client of an existing executor (that way someone could use the new_with_options function to create a functional executor, then modify the client to fit their needs) (ie, make a new function that acheives what I think the for_client function is supposed to do)
• add modify the for_client function to return an executor that implements the Executor trait

I could probably make a PR for any one of those solutions, the real question is which one, if any, is most in line with the goals of this project

[AnthonyMichaelTDM]

more in depth review of the options:

• the most ergonomic approach is changing llm_chain_openai::chatgpt::options::PerExecutor to be something like this:

  pub struct PerExecutor {
  pub api_key: Option<String>,
  pub organization_id: Option<String>,
  }

  but that would be a breaking change,

• we could also change llm_chain_openai::chatgpt::Executor::for_client from

  pub fn for_client(
  client: async_openai::Client,
  per_invocation_options: Option<PerInvocation>,
  ) -> Self {
  let client = Arc::new(client);
  Self {
      client,
      per_invocation_options,
  }
  }

  to

  pub fn for_client(
  mut self,
  client: async_openai::Client,
  ) -> Self {
  self.client = Arc::new(client);
  self
  }

  but this is also technically a breaking change because it changes the functionality of a (admittedly broken) public function

• last option I'm considering is to add a new function, with_client to llm_chain_openai::chatgpt::Executor

pub fn with_client(
    mut self,
    client: async_openai::Client,
) -> Self {
    self.client = Arc::new(client);
    self
}

Summary:

realistically, unless you're planning a major release soon, option 1 is too destructive to existing functionality, so that leaves 2 and 3,

IF the for_client function is currently operating how it is intended, then I should do option 3, otherwise option 2 because this is a bug.

So, final question: is the for_client function currently operating as is intended?

[AnthonyMichaelTDM]

option 4, probably the best, will make a PR with this tomorrow if I don't get any feedback:

change the for_client function to something like this:

    pub fn for_client(
        client: async_openai::Client,
        per_invocation_options: Option<PerInvocation>,
    ) -> Self {
        use llm_chain::traits::Executor as _;
        let mut exec = Self::new_with_options(None, per_invocation_options).unwrap;
        exec.client = Arc::new(client);
        exec
    }

maintains documented behavior, unless I'm missing something this is probably the best