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

fix(auth): simplify provider handling with URL-based configuration #465

Merged
merged 9 commits into from
Mar 7, 2025

Conversation

tusharmath
Copy link
Collaborator

@tusharmath tusharmath commented Mar 7, 2025

This PR refactors the authentication system to use URL-based provider detection instead of enum-based configuration, providing more flexibility in supporting different provider endpoints.

Changes

  • Refactor Provider enum to directly store URLs for OpenAI-compatible providers
  • Simplify environment configuration and provider detection
  • Remove tests related to the old provider implementation
  • Add URL serialization support via serde
  • Refactor the client builder to work with the new provider model
  • Remove force_antinomy feature flag from environment configurations
  • Remove login and logout commands from the available commands list, as they are no longer needed

Testing

The implementation preserves backward compatibility while adding new functionality.

- Refactor provider service to support multiple authentication methods\n- Add support for Antinomy provider\n- Update OpenRouter URL from api.openrouter.io to openrouter.ai\n- Improve provider architecture with generics\n- Add force_antinomy flag for environment configuration\n- Restructure client builder pattern for better type safety
@tusharmath tusharmath changed the title feat(auth): integrate authentication v2 with Antinomy support refactor(auth): simplify provider handling with URL-based configuration Mar 7, 2025
- Remove serde feature from url dependency

- Remove Serialize, Deserialize and From derives from Provider enum

- Extract provider resolution logic to a separate method
@@ -33,14 +33,24 @@ impl ForgeEnvironmentService {
fn get(&self) -> Environment {
dotenv::dotenv().ok();
let cwd = std::env::current_dir().unwrap_or(PathBuf::from("."));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provider can store the key.

@tusharmath
Copy link
Collaborator Author

Added a new commit to further simplify the Provider implementation:

  • Removes serde feature from url dependency
  • Removes Serialize, Deserialize and From derives from Provider enum
  • Extracts provider resolution logic to a separate method in environment service

This continues the refactoring work to streamline the authentication system.

)),

forge_domain::Provider::Anthropic => Ok(Client::Anthropic(
Anthropic::builder().api_key(api_key).build()?,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a derived builder.

- Restructure Provider enum to include API keys in its variants
- Remove dependency on Infrastructure in ForgeProviderService
- Update Environment to store Provider instance instead of separate URL and key
- Add serde feature to URL dependency for serialization support
- Update tests to use the new Provider structure
- Simplify client builder to get API key directly from Provider
…r handling

- Remove mutex and lazy initialization in ForgeProviderService
- Add key() and is_antinomy() helper methods to Provider struct
- Replace ClientBuilder with simpler Client::new() constructor
- Update AnthropicBuilder to use derive_builder::Builder
- Improve URL handling in OpenRouter implementation
}
if self.provider.is_open_router() | self.provider.is_antinomy() {
// For Eg: https://openrouter.ai/api/v1/parameters/google/gemini-pro-1.5-exp
let path = format!("parameters/{}", model.as_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let path = format!("parameters/{}", model.as_str());
let path = if self.provider.is_antinomy() {
format!("model/{}/parameters", model.as_str())
} else {
format!("parameters/{}", model.as_str())
};

@tusharmath tusharmath enabled auto-merge (squash) March 7, 2025 10:35
@tusharmath tusharmath disabled auto-merge March 7, 2025 10:35
@tusharmath tusharmath changed the title refactor(auth): simplify provider handling with URL-based configuration fix(auth): simplify provider handling with URL-based configuration Mar 7, 2025
@tusharmath tusharmath enabled auto-merge (squash) March 7, 2025 10:35
@tusharmath tusharmath merged commit 2a369a2 into main Mar 7, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants