Pass req_opts to model calls#351
Conversation
|
Hi @nallwhy! Thanks for taking the initiative to make this happen. I reviewed your description in #347 and you point out the following: I see a few possible ways to address this:
And here you opted for approach 3. I really appreciate your considerations on which approach makes the most sense. In this situation, I actually favor approach 2. Here's why: A ChatModel is the thing that talks to the LLM for you. It isn't exclusively about the LLM's model config. Internally, it uses Req. It's appropriate for Req request options to be exposed on the struct and be used in the request. Approach 2 is also a much cleaner change that doesn't risk breaking so many internal functions. If we expect more types of request related config to be needed, then I could creating a Req config struct so the config could be standardized and struct defined functions could be used to apply the config. Example of what I mean: req =
Req.new(
url: url(anthropic),
json: raw_data,
headers: headers(anthropic),
receive_timeout: anthropic.receive_timeout,
retry: :transient,
max_retries: 3,
retry_delay: fn attempt -> 300 * attempt end,
aws_sigv4: aws_sigv4_opts(anthropic.bedrock)
)
|> ReqConfig.apply_options(anthropic.req_config) # <-- something like this lineIf the options is the only config you can see as applicable, then we can keep it simple and just do the options. What do you think? |
|
I see your point, and I agree. As you suggested, I’ll add |
This PR resolves #347.