Maint/depcheck#525
Open
viseshrp wants to merge 5 commits into
Open
Conversation
Introduce a dedicated serverless dependency-check helper that keeps backwards-compatibility enforcement out of ADR.setup(). The helper embeds the 261 compatibility profile for the three intentionally upgraded libraries (django, django-guardian, and djangorestframework) so runtime enforcement remains reliable from an installed wheel without depending on repo-local constraints files. The implementation is intentionally narrow: install version 261 gets exact runtime enforcement, while newer/current lines bypass strict auditing in this module. This keeps the policy aligned with the branch goal of protecting backward compatibility for older ADR installs rather than attempting a broad environment audit. Add focused unit coverage for: - keeping the embedded 261 profile in sync with constraints/v261.txt - skipping enforcement for non-261 installs - reporting missing and out-of-range dependencies for 261 - raising ImproperlyConfiguredError only when the 261 profile is violated
Wire the existing serverless dependency checker into ADR.setup() so older 261 installs are validated before product Django settings are imported. This keeps the setup method thin by delegating the compatibility policy to serverless/_dep_check.py, while changing the setup failure mode from a later import/configuration traceback to an early ImproperlyConfiguredError with a targeted compatibility message. The test coverage added here is intentionally narrow and integration-focused. It builds the smallest valid 261 install layout, patches the dependency checker to fail, and verifies that setup surfaces that failure before it ever tries to import ceireports.settings_serverless. The test also restores the ADR singleton state explicitly so it does not leak global state into the rest of the serverless test suite. Validation run for this commit: - .venv\\Scripts\\python -m pytest tests/serverless/test_dep_check.py tests/serverless/test_adr.py -k "dep_check or dependency or setup_enforces_runtime_dependencies_before_importing_settings" - .venv\\Scripts\\python -m ruff check src/ansys/dynamicreporting/core/serverless/_dep_check.py src/ansys/dynamicreporting/core/serverless/adr.py tests/serverless/test_dep_check.py tests/serverless/test_adr.py - uv run python tests/smoketest.py
Extend the centralized serverless dependency checker with two policy layers that match the branch compatibility story. First, keep the existing exact v261 compatibility profile and upgrade its failure message to point users at the project's constraints/v261.txt compatibility set. This preserves the strict backwards-compatibility guard for the older line while giving a clearer remediation path than a generic mismatch error. Second, add a broader supported dependency range profile that mirrors the branch package policy in pyproject.toml for the monitored upgraded libraries: django, django-guardian, and djangorestframework. Versions outside that tested range now produce a warning for non-v261 installs instead of a hard failure, because current-line setup still uses the best-effort compatibility shims in serverless/_compat.py. The implementation stays fully inside serverless/_dep_check.py so ADR.setup() does not grow additional policy branches. The audit still does a single installed-version scan and linear window comparisons, and the new tests keep the embedded runtime profiles synchronized with both constraints/v261.txt and pyproject.toml. Validation run for this commit: - .venv\\Scripts\\python -m pytest tests/serverless/test_dep_check.py tests/serverless/test_adr.py -k "dep_check or dependency or setup_enforces_runtime_dependencies_before_importing_settings" - .venv\\Scripts\\python -m ruff check src/ansys/dynamicreporting/core/serverless/_dep_check.py tests/serverless/test_dep_check.py tests/serverless/test_adr.py - uv run python tests/smoketest.py
Refactor serverless/_compat.py from a single sanitize_settings pass into a phased compatibility planner that can support future compatibility fixes without pushing more policy into ADR.setup(). The new planner introduces explicit pre-configure, post-configure, and pre-setup phases. The existing guardian and storage migrations remain pre-configure rules, so current behavior is preserved. The refactor also adds an installed-app filtering helper that preserves app order and container type, giving the compatibility layer a reusable and deterministic way to apply future INSTALLED_APPS filtering rules. This commit keeps ADR.setup() unchanged on purpose. It only adds the centralized engine, deferred-hook data structures, compatibility-plan builder, and unit tests that prove: - current sanitize_settings behavior still works - deferred hooks are collected by phase in deterministic order - sanitize_settings remains a backwards-compatible wrapper that does not execute deferred hooks - installed-app filtering support is available without module stubbing Validation run for this commit: - .venv\\Scripts\\python -m pytest tests/serverless/test_compat.py - .venv\\Scripts\\python -m ruff check src/ansys/dynamicreporting/core/serverless/_compat.py tests/serverless/test_compat.py - uv run python tests/smoketest.py
Wire the new phased compatibility planner into ADR.setup() while keeping the setup method thin. ADR.setup() now asks _compat.py for a compatibility plan, uses the rewritten overrides for settings.configure(), then runs deferred post-configure and pre-setup hooks before calling django.setup(). This change keeps the compatibility logic centralized in _compat.py instead of expanding core setup code with more conditionals. The ADR-side diff is intentionally minimal and only performs phase orchestration. The integration test added here builds a minimal temporary ADR install, schedules synthetic post-configure and pre-setup hooks through the real _compat registries, and proves the execution order is: - post-configure hooks - pre-setup hooks - django.setup() That verifies the new hook phases are actually honored by ADR.setup(), not just unit-tested in isolation. The implementation also satisfies the remaining no-stubbing requirement because it uses explicit rewrites and deferred hooks only; no module stubs were introduced. Validation run for this commit: - .venv\\Scripts\\python -m pytest tests/serverless/test_compat.py tests/serverless/test_adr.py -k "setup_runs_phased_compatibility_hooks_before_django_setup or setup_enforces_runtime_dependencies_before_importing_settings or test_sanitize_settings or test_filter_installed_apps or test_build_compatibility_plan or test_get_installed_versions_returns_real_packages or test_normalize_version_ignores_non_numeric_suffixes or test_guardian_rename_is_noop_when_old_key_absent or test_storage_migration_is_noop_when_old_key_absent" - .venv\\Scripts\\python -m ruff check src/ansys/dynamicreporting/core/serverless/_compat.py src/ansys/dynamicreporting/core/serverless/adr.py tests/serverless/test_compat.py tests/serverless/test_adr.py - uv run python tests/smoketest.py
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a serverless runtime dependency audit/enforcement step (strict for ADR v261, warn-only for newer lines) and refactors the serverless settings compatibility shim into a phased “plan + deferred hooks” system, with accompanying tests to lock in behavior and ordering during ADR.setup().
Changes:
- Introduce
core.serverless._dep_checkto audit/enforce monitored dependency versions and surface actionable guidance for v261. - Refactor
core.serverless._compatinto a phased compatibility hook system (build_compatibility_plan, post-configure hooks, pre-setup hooks) and updateADR.setup()orchestration. - Add tests covering dependency window sync, audit/enforcement behavior, phased hook planning/execution, and
ADR.setup()call ordering.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/serverless/test_dep_check.py | New unit tests for dependency window sync + audit/enforce behavior across install lines. |
| tests/serverless/test_compat.py | New tests for phased compatibility plan + hook execution and helper behavior. |
| tests/serverless/test_adr.py | New tests asserting dep-check happens before product settings import, and phased hooks run before django.setup(). |
| src/ansys/dynamicreporting/core/serverless/adr.py | Enforce runtime deps early and orchestrate phased compatibility hooks around Django configuration/setup. |
| src/ansys/dynamicreporting/core/serverless/_dep_check.py | New dependency audit/enforcement implementation with strict v261 policy + warn-only supported range. |
| src/ansys/dynamicreporting/core/serverless/_compat.py | Hook-system refactor; keep sanitize_settings as a backwards-compatible wrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+199
to
+204
| def _version_in_window( | ||
| installed_version: InstalledDependencyVersion, expected_window: DependencyWindow | ||
| ) -> bool: | ||
| """Return ``True`` when the installed version fits the exact compatibility window.""" | ||
| normalized = installed_version.normalized_version | ||
| return expected_window.minimum_version <= normalized < expected_window.maximum_version |
There was a problem hiding this comment.
Version window comparisons can be wrong when the installed version string omits patch components (e.g., Django releases like "6.0" normalize to (6, 0)). With the current tuple comparison, (6, 0) is treated as < (6, 0, 0), so an installed version that is effectively 6.0.0 may incorrectly be considered inside a "<6.0.0" window and skip the intended warning. Consider normalizing to a fixed-length tuple (e.g., pad missing components with zeros) or otherwise aligning tuple lengths before comparing in _version_in_window.
Suggested change
| def _version_in_window( | |
| installed_version: InstalledDependencyVersion, expected_window: DependencyWindow | |
| ) -> bool: | |
| """Return ``True`` when the installed version fits the exact compatibility window.""" | |
| normalized = installed_version.normalized_version | |
| return expected_window.minimum_version <= normalized < expected_window.maximum_version | |
| def _pad_version_key(version: VersionKey, length: int) -> VersionKey: | |
| """Return a version tuple padded with trailing zeros to ``length`` items.""" | |
| if len(version) >= length: | |
| return version | |
| return version + (0,) * (length - len(version)) | |
| def _version_in_window( | |
| installed_version: InstalledDependencyVersion, expected_window: DependencyWindow | |
| ) -> bool: | |
| """Return ``True`` when the installed version fits the exact compatibility window.""" | |
| normalized = installed_version.normalized_version | |
| comparison_length = max( | |
| len(normalized), | |
| len(expected_window.minimum_version), | |
| len(expected_window.maximum_version), | |
| ) | |
| normalized = _pad_version_key(normalized, comparison_length) | |
| minimum_version = _pad_version_key(expected_window.minimum_version, comparison_length) | |
| maximum_version = _pad_version_key(expected_window.maximum_version, comparison_length) | |
| return minimum_version <= normalized < maximum_version |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.