Improve provider validation ImportError messages with root cause details#7404
Improve provider validation ImportError messages with root cause details#7404biefan wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@biefan please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
alvinttang
left a comment
There was a problem hiding this comment.
Straightforward improvement — surfacing the root-cause ImportError message is genuinely helpful for users debugging missing dependencies like vertexai, azure-identity, etc.
A couple of notes:
-
validation_service.py: The change looks correct. Theexcept ImportError as enow captures the original exception and includes{e}in the error string. One consideration: in rare cases,ImportErrorcan be chained (e.g.,ImportErrorcaused by anotherImportErrorduring transitive imports). You might want to also includee.__cause__if present, e.g.:root = f" (caused by: {e.__cause__})" if e.__cause__ else "" error=f"Could not import provider {provider}: {e}{root}"
This would help when the immediate error is "No module named 'autogen_ext.models.openai'" but the root cause is actually a missing
openaipackage. -
Test (
test_validation_service.py): Good that you added a test. The monkeypatch onimportlib.import_moduleis clean. One edge case: the test asserts"Could not import provider autogen_ext.models.openai.OpenAIChatCompletionClient"— this relies on the_resolve_provider_pathlogic prepending the package prefix. If that logic changes, this test would break for non-obvious reasons. Consider adding a comment noting this coupling. -
Suggestion wording: The updated suggestion
"Check that the provider module and its optional dependencies are installed, then verify the import path."is an improvement. You could make it even more actionable:"... are installed (e.g., pip install 'autogen-ext[openai]'), then verify ..."— but that might be too specific for a generic handler.
Nice contribution.
|
@microsoft-github-policy-service agree |
When optional dependency imports fail, users currently see only a generic 'Could not import provider X' message without the original exception details (e.g., 'No module named vertexai'). This change: - Captures the original ImportError exception - Includes it in the error message for better debugging - Updates suggestion to mention optional/transitive dependencies Fixes microsoft#7404
Summary
Improve
ValidationService.validate_provider()so provider validation errors include the originalImportErrordetails instead of only a generic message.Changes
No module named 'vertexai').Why
When optional dependency imports fail, users currently only see
Could not import provider ...and lose the root cause. Including the original error makes troubleshooting much faster.Tests
uv run pytest -q tests/test_validation_service.pyuv run ruff check autogenstudio/validation/validation_service.py tests/test_validation_service.pyFixes #4605