feat: add Ollama, Azure OpenAI, and generic OpenAI-compatible providers - #179
Conversation
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[Automated SkillSpector Review]
Approved.
Three providers (Ollama, Azure OpenAI, generic OpenAI-compatible) following the established provider pattern. Credential resolution fails closed (Azure and the generic provider return None unless both env vars are set; Ollama supplies the required placeholder key), the Azure key is wrapped in SecretStr, and each ships a bundled model_registry.yaml. 35 tests cover credentials (positive/negative), chat-model creation, metadata, selection, and the updated error message.
Security: the configurable base URLs (OLLAMA_BASE_URL, SKILLSPECTOR_COMPAT_BASE_URL, AZURE_OPENAI_ENDPOINT) are operator-set env vars — the same trust surface as the existing OPENAI_BASE_URL — so no new attacker-controlled SSRF vector from scanned content. Non-blocking: could validate that the base URL is a well-formed http(s) URL to fail earlier on misconfiguration.
Add DEFAULT_MODEL and SLOT_DEFAULTS as ClassVar declarations to ModelMetadataProvider Protocol so callers no longer need type: ignore[attr-defined] when accessing provider attributes. Add validate_base_url() helper that warns on malformed (non-http/https or missing host) base URLs at model creation time, catching operator misconfigurations early without raising. Remove stale type: ignore[attr-defined] from constants.py. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Add DEFAULT_MODEL and SLOT_DEFAULTS as ClassVar declarations to ModelMetadataProvider Protocol so callers no longer need type: ignore[attr-defined] when accessing provider attributes. Add validate_base_url() helper that warns on malformed (non-http/https or missing host) base URLs at model creation time, catching operator misconfigurations early without raising. Remove stale type: ignore[attr-defined] from constants.py. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com> (cherry picked from commit d0006e1)
7f4296a to
46f3a9d
Compare
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[Automated SkillSpector Review]
Re-review: approved. The rewritten head preserves the previously approved Ollama, Azure OpenAI, and generic OpenAI-compatible providers while integrating current main’s Bedrock and agent-CLI options in selection/error text. Credential resolution remains fail-closed where required, secrets use the native/shared constructors, registries remain provider-local, and focused coverage spans selection, credentials, construction, metadata, defaults, and errors.
|
@mimran-khan - Please fix CI issues |
Ollama provider enables free local/offline LLM scanning via Ollama's OpenAI-compatible API at localhost:11434. No API key required. Azure OpenAI provider supports enterprise deployments using AzureChatOpenAI with deployment-based routing and api-version handling. Generic OpenAI-compatible provider serves Groq, Together AI, Mistral, DeepInfra, and other endpoints with dedicated env vars (SKILLSPECTOR_COMPAT_API_KEY/BASE_URL) and a bundled multi-provider model registry. Each provider ships its own model_registry.yaml for accurate token budgeting. Closes NVIDIA#173, closes NVIDIA#174, closes NVIDIA#175 Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
46f3a9d to
e321988
Compare
|
Rebased, added the DCO sign-off and fixed the ruff format issue. Should be green now. @rng1995 |
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
Re-review approved at the current head. The previously reviewed provider behavior remains acceptable and there are no unresolved review threads. CI is green except for the mechanical DCO failure on the unsigned merge commit.
1e1a92d to
ea0cb18
Compare
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
ff2dafd to
2d28425
Compare
2 participants
Summary
Add three new LLM providers to expand SkillSpector beyond cloud-only OpenAI/Anthropic/NVIDIA endpoints:
SKILLSPECTOR_PROVIDER=ollama) — free local/offline scanning via Ollama's OpenAI-compatible API. No API key required. Supports remote instances viaOLLAMA_BASE_URL.SKILLSPECTOR_PROVIDER=azure_openai) — enterprise Azure-hosted deployments usingAzureChatOpenAIwith deployment-based routing,api-versionhandling, and dedicated env vars (AZURE_OPENAI_ENDPOINT,AZURE_OPENAI_API_KEY,AZURE_OPENAI_DEPLOYMENT).SKILLSPECTOR_PROVIDER=openai_compatible) — Groq, Together AI, Mistral, DeepInfra, Fireworks, and any OpenAI-compatible endpoint viaSKILLSPECTOR_COMPAT_API_KEY+SKILLSPECTOR_COMPAT_BASE_URL.Each provider ships its own
model_registry.yamlfor accurate token budgeting.Changes
src/skillspector/providers/ollama/— provider, model registry,__init__src/skillspector/providers/azure_openai/— provider, model registry,__init__src/skillspector/providers/openai_compatible/— provider, model registry,__init__src/skillspector/providers/__init__.py— register new providers in selector, update error message to mentionollamatests/unit/test_new_providers.py— 35 tests covering credentials, chat model creation, metadata, selection, and error messagesCloses #173, closes #174, closes #175
Test plan
pytest tests/unit/test_new_providers.py -v)OLLAMA_BASE_URLAZURE_OPENAI_API_KEYandAZURE_OPENAI_ENDPOINTAZURE_OPENAI_DEPLOYMENTunset2024-06-01SKILLSPECTOR_COMPAT_API_KEYandSKILLSPECTOR_COMPAT_BASE_URL