Refactor Nexity authentication to use inline lazy imports#1972
Refactor Nexity authentication to use inline lazy imports#1972
Conversation
|
@copilot is this the best way to lazy load these packages, that are only used for authentication of Nexity (which isn't used a lot). |
There was a problem hiding this comment.
Pull request overview
This PR refactors Nexity authentication so its third-party dependencies (boto3 / botocore / warrant_lite) are imported dynamically at runtime rather than at module import time, and updates tests to patch the new loader.
Changes:
- Added
_load_nexity_auth_dependencies()to import Nexity-specific libraries on demand and raise a Nexity-specific exception on missing deps. - Updated
NexityAuthStrategy.login()to use the dynamically loaded modules/classes. - Updated Nexity auth tests to patch
_load_nexity_auth_dependencies()instead of patching module-level imports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
pyoverkiz/auth/strategies.py |
Introduces a dynamic dependency loader and rewires Nexity login to use it. |
tests/test_auth.py |
Adjusts Nexity auth error-mapping tests to patch the new loader return values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pyoverkiz/auth/strategies.py
Outdated
| except ImportError as error: | ||
| raise NexityServiceException( | ||
| "Nexity authentication requires optional dependencies. Install with the " | ||
| "Nexity auth extras." | ||
| ) from error |
There was a problem hiding this comment.
The error message here references installing "Nexity auth extras", but the project currently lists boto3 and warrant-lite as required dependencies in pyproject.toml (not optional), and there is no [project.optional-dependencies] entry for Nexity. Either define a real extras group and move these deps out of the base dependencies, or update the message to instruct installing the specific packages (or the correct extras name).
pyoverkiz/auth/strategies.py
Outdated
| def _load_nexity_auth_dependencies() -> tuple[Any, Any, type[Exception], Any]: | ||
| """Load Nexity optional dependencies only when Nexity auth is used.""" | ||
| try: | ||
| boto3_module = importlib.import_module("boto3") | ||
| botocore_config_module = importlib.import_module("botocore.config") | ||
| botocore_exceptions_module = importlib.import_module("botocore.exceptions") | ||
| warrant_lite_module = importlib.import_module("warrant_lite") | ||
| except ImportError as error: | ||
| raise NexityServiceException( | ||
| "Nexity authentication requires optional dependencies. Install with the " | ||
| "Nexity auth extras." | ||
| ) from error | ||
|
|
||
| return ( | ||
| boto3_module, | ||
| botocore_config_module.Config, | ||
| cast(type[Exception], botocore_exceptions_module.ClientError), | ||
| warrant_lite_module.WarrantLite, | ||
| ) |
There was a problem hiding this comment.
_load_nexity_auth_dependencies() returns an unstructured 4-tuple of Any, which makes call sites easy to mis-order and obscures what each value represents. Consider returning a NamedTuple/dataclass (or a small object) with named fields and tighter types (e.g., types.ModuleType for boto3_module, the specific Config/ClientError/WarrantLite types) to keep the refactor readable and type-checker friendly.
pyoverkiz/auth/strategies.py
Outdated
| loop = asyncio.get_running_loop() | ||
| boto3_module, config_class, client_error_class, warrant_lite_class = ( | ||
| _load_nexity_auth_dependencies() | ||
| ) | ||
|
|
||
| def _client() -> BaseClient: | ||
| return boto3.client( | ||
| "cognito-idp", config=Config(region_name=NEXITY_COGNITO_REGION) | ||
| def _client() -> Any: | ||
| return boto3_module.client( | ||
| "cognito-idp", config=config_class(region_name=NEXITY_COGNITO_REGION) |
There was a problem hiding this comment.
importlib.import_module(...) runs synchronously on the event loop the first time Nexity login is used, which can briefly block the loop due to filesystem I/O during imports. Consider caching the loaded dependencies (module-level cache) and/or running the dependency loading in the executor along with the boto3 client creation to avoid blocking async workloads.
pyoverkiz/auth/strategies.py
Outdated
| try: | ||
| boto3_module = importlib.import_module("boto3") | ||
| botocore_config_module = importlib.import_module("botocore.config") | ||
| botocore_exceptions_module = importlib.import_module("botocore.exceptions") | ||
| warrant_lite_module = importlib.import_module("warrant_lite") | ||
| except ImportError as error: | ||
| raise NexityServiceException( | ||
| "Nexity authentication requires optional dependencies. Install with the " | ||
| "Nexity auth extras." | ||
| ) from error |
There was a problem hiding this comment.
The new ImportError-to-NexityServiceException mapping in _load_nexity_auth_dependencies() is new behavior and currently has no test coverage. Adding a test that forces _load_nexity_auth_dependencies() to raise ImportError (and asserts NexityServiceException with a helpful message) would prevent regressions and validate the intended UX.
…login() Agent-Logs-Url: https://github.com/iMicknl/python-overkiz-api/sessions/48de7731-1347-4517-9e5e-aa296278a067 Co-authored-by: iMicknl <1424596+iMicknl@users.noreply.github.com>
The The inline import approach has several advantages over the
Refactored in commit |
Refactors the Nexity authentication strategy to load optional dependencies lazily at runtime using standard inline
importstatements inside thelogin()method, rather than importing them at the module level. This keeps the code readable while avoiding unnecessary imports unless Nexity authentication is actually used.Dependency loading improvements:
boto3,botocore.config,botocore.exceptions, andwarrant_litefrom the module level into the body ofNexityAuthStrategy.login(), using atry/except ImportErrorblock to raise a clearNexityServiceExceptionif the optional dependencies are missing._load_nexity_auth_dependencieshelper function and theimport importlibmodule-level import, restoring full type information (noAnyorcastworkarounds).Testing updates:
boto3.clientandwarrant_lite.WarrantLitedirectly, which is the correct approach when the imports live inside the method body rather than at module level.