🧹 Refactor duplicated Import Error boilerplate in LLM Providers#32
🧹 Refactor duplicated Import Error boilerplate in LLM Providers#32
Conversation
- Created a `validate_import` decorator in `llm_provider.py` to handle `ImportError` for optional dependencies. - Applied the decorator to all `LLMProvider` subclasses, removing duplicated `try-except` blocks. - Added a new test file `tests/test_llm_provider_imports.py` to verify the standardized error messages, using `unittest.mock.patch.dict` to simulate missing modules. - Verified that existing tests still pass.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request introduces a validate_import decorator in llm_provider.py to standardize the handling of ImportError for optional LLM provider dependencies. Several provider classes, including GoogleProvider, OllamaProvider, and OpenAIProvider, have been refactored to use this decorator, which simplifies their initialization logic by removing repetitive try-except blocks. Additionally, a new test suite tests/test_llm_provider_imports.py has been added to verify that the correct error messages are raised when these optional dependencies are missing. I have no feedback to provide.
This PR improves the maintainability and readability of the LLM provider implementations by standardizing how optional dependency imports are handled.
🎯 What:
validate_import(extra_name)decorator inpdf_anonymizer_core.llm_provider.try...except ImportErrorblocks inGoogleProvider,OllamaProvider,HuggingFaceProvider,OpenRouterProvider,OpenAIProvider, andAnthropicProviderwith the new decorator.💡 Why:
The previous implementation had a lot of boilerplate code for each provider to check if its required package was installed. This refactoring centralizes that logic, making it easier to add new providers and ensuring a consistent user experience when an optional extra is missing.
✅ Verification:
tests/test_llm_provider_imports.pyverify that the correctImportErrorwith the expected message is raised for each provider when its module is unavailable. These tests mocksys.modulesto be environment-independent.tests/test_main.py(using mocks for the LLM calls) continue to pass.✨ Result:
Cleaner, more maintainable code with a standardized pattern for handling optional dependencies across all LLM providers.
PR created automatically by Jules for task 11010072440898765876 started by @leo-gan