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

Add OpenAI URL override and support for custom request headers #27

Closed
wants to merge 1 commit into from

Conversation

aspires
Copy link

@aspires aspires commented Mar 16, 2025

Relates to #9

Preface
Opening early to discuss whether this is a welcome addition, the architectural decisions, documentation implications, and whether expanding this URL config override to other LLMs within the gem's scope makes sense. I'm committed to maintaining the developer experience simplicity of the project's stated mission, so I'm on board with redoing the initial approach to fit in with longer-term priorities (or closing outright!).


Goal
This commit introduces the configuration primitives necessary for semantic caching proxy services for OpenAI requests. Specifically, the option to override the OpenAI root API URL path and the ability to add custom headers to the request payload in the form of a hash. I'm a Fastly employee, and I used our semantic caching product as a test implementation.


In-Progress Disclaimer
This branch does not yet introduce tests and should not be merged until a subsequent commit lands with proper testing.

This commit introduces the configuration primitives necessary for
semantic caching proxy services for OpenAI requests.Specifically,
the ability to override the OpenAI root API URL path and the
ability to add custom headers to the request payload in the form
of a hash.

This commit does not introduce tests, and should not be merged
until a subsequent commit lands with proper testing.
@crmne crmne added the enhancement New feature or request label Mar 21, 2025
@crmne
Copy link
Owner

crmne commented Mar 23, 2025

I appreciate the thought that went into this PR, but I'd rather not go down the path of generic URL overrides and custom headers. That's essentially creating a backdoor that undermines our provider abstraction.

Instead, I'd much prefer we implement a proper RubyLLM::Provider::Fastly that handles the semantic caching natively. This would give us:

  1. A clean implementation that doesn't expose HTTP details
  2. The ability to properly document the Fastly integration
  3. Proper pricing and capabilities integration

The "override URL and add headers" approach feels like a shortcut that would introduce significant technical debt. We'd be passing responsibility for correct implementation to users rather than handling it properly in the library.

Since you work at Fastly, you're in a perfect position to implement this provider the right way. It might be slightly more code than URL overrides, but it would be a much more robust and maintainable solution in the long run.

Let's close this PR and create a new issue specifically for adding Fastly provider support. That's a contribution I'd be excited to see!

@crmne crmne closed this Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants