Skip to content

Commit 5d1ccd5

Browse files
committed
Forever ensure that ansible_base.lib.* is importable
1 parent 829846e commit 5d1ccd5

File tree

6 files changed

+192
-16
lines changed

6 files changed

+192
-16
lines changed

.github/workflows/linting.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ jobs:
2222
command: check_black
2323
- name: api-isort
2424
command: check_isort
25+
- name: pure-python-imports
26+
command: check_pure_python_imports
2527
steps:
2628
- name: Install make
2729
run: sudo apt install make

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ COMPOSE_UP_OPTS ?=
1111
DOCKER_COMPOSE ?= docker compose
1212

1313
.PHONY: PYTHON_VERSION clean build\
14-
check lint check_black check_flake8 check_isort git_hooks_config
14+
check lint check_black check_flake8 check_isort check_pure_python_imports git_hooks_config
1515

1616
PYTHON_VERSION:
1717
@echo "$(subst python,,$(PYTHON))"
@@ -49,6 +49,10 @@ check_flake8:
4949
check_isort:
5050
tox -e isort -- --check $(CHECK_SYNTAX_FILES)
5151

52+
## Check that lib modules can be imported with pure Python (no Django setup)
53+
check_pure_python_imports:
54+
$(PYTHON) check_pure_python_imports.py
55+
5256
## Starts a postgres container in the background if one is not running
5357
# Options:
5458
# -d, --detatch: run the container in background
Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1-
from django import template
2-
31
from ansible_base.lib.utils import requests as dab_requests
42

5-
register = template.Library()
3+
# Global variable to hold the Django template library
4+
register = None
65

76

8-
@register.simple_tag
97
def is_proxied_request():
108
return dab_requests.is_proxied_request()
9+
10+
11+
# Auto-register when imported in Django context
12+
try:
13+
from django import template
14+
register = template.Library()
15+
register.simple_tag(is_proxied_request)
16+
except ImportError:
17+
# Django not available, defer registration
18+
pass

ansible_base/lib/utils/create_system_user.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
1-
import logging
2-
from typing import Optional, Tuple, Type
1+
from __future__ import annotations
32

4-
from django.core.exceptions import ImproperlyConfigured
5-
from django.db import models
6-
from django.utils.translation import gettext as _
3+
import logging
4+
from typing import TYPE_CHECKING, Optional, Tuple, Type
75

86
from ansible_base.lib.utils.settings import get_setting
97

8+
if TYPE_CHECKING:
9+
from django.db import models
10+
1011
logger = logging.getLogger('ansible_base.lib.utils.create_system_user')
1112

1213
"""
1314
These functions are in its own file because it is loaded during migrations so it has no access to models.
1415
"""
1516

1617

17-
def create_system_user(user_model: Type[models.Model]) -> models.Model: # Note: We can't load models here so we can typecast to anything better than Model
18+
def create_system_user(user_model: Type[models.Model]) -> models.Model:
19+
# Note: We can't load models here so we can typecast to anything better than Model
1820
from ansible_base.lib.abstract_models.user import AbstractDABUser
1921

2022
#
@@ -63,4 +65,7 @@ def get_system_username() -> Tuple[Optional[str], str]:
6365
return str(value), setting_name
6466

6567
logger.error(f"Expected get_setting to return a string for {setting_name}, got {type(value)}")
68+
69+
from django.core.exceptions import ImproperlyConfigured
70+
from django.utils.translation import gettext as _
6671
raise ImproperlyConfigured(_("Setting %(setting_name)s needs to be a string not a %(type)s") % {'setting_name': setting_name, 'type': type(value)})

ansible_base/lib/utils/requests.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import logging
2-
from typing import Optional
1+
from __future__ import annotations
32

4-
from crum import get_current_request
5-
from django.http import HttpRequest
3+
import logging
4+
from typing import TYPE_CHECKING, Optional
65

7-
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
86
from ansible_base.lib.utils.settings import get_setting
97

8+
if TYPE_CHECKING:
9+
from django.http import HttpRequest
10+
1011
logger = logging.getLogger('ansible_base.lib.uitls.requests')
1112

1213

@@ -43,6 +44,8 @@ def get_remote_hosts(request: HttpRequest, get_first_only: bool = False) -> list
4344
# If we are connected to from a trusted proxy then we can add some additional headers
4445
try:
4546
if 'HTTP_X_TRUSTED_PROXY' in request.META:
47+
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
48+
4649
if validate_x_trusted_proxy_header(request.META['HTTP_X_TRUSTED_PROXY']):
4750
# The last entry in x-forwarded-for from envoy can be trusted implicitly
4851
# https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#x-forwarded-for
@@ -68,10 +71,14 @@ def get_remote_hosts(request: HttpRequest, get_first_only: bool = False) -> list
6871
def is_proxied_request(request: Optional[HttpRequest] = None) -> bool:
6972
"Return true if request claims to be from a proxy and the header validates as such."
7073
if request is None:
74+
from crum import get_current_request
75+
7176
request = get_current_request()
7277
if request is None:
7378
# e.g. being called by CLI or something
7479
return False
7580
if x_trusted_proxy := request.META.get("HTTP_X_TRUSTED_PROXY"):
81+
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
82+
7683
return validate_x_trusted_proxy_header(x_trusted_proxy)
7784
return False

check_pure_python_imports.py

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
#!/usr/bin/env python3
2+
# Generated by Claude Code (Sonnet 4)
3+
4+
"""
5+
Script to test all modules in ansible_base/lib for pure Python compatibility.
6+
This script ensures that modules in the lib folder can be imported without
7+
requiring Django apps to be initialized.
8+
"""
9+
10+
import os
11+
import sys
12+
import subprocess
13+
import importlib.util
14+
from pathlib import Path
15+
16+
def find_python_modules(lib_path):
17+
"""Find all Python modules in the lib directory."""
18+
modules = []
19+
lib_path = Path(lib_path)
20+
21+
for py_file in lib_path.rglob("*.py"):
22+
if py_file.name == "__init__.py":
23+
# Convert path to module name (including ansible_base prefix)
24+
relative_path = py_file.parent.relative_to(lib_path.parent.parent)
25+
module_name = str(relative_path).replace(os.sep, ".")
26+
modules.append(module_name)
27+
else:
28+
# Convert path to module name (including ansible_base prefix)
29+
relative_path = py_file.relative_to(lib_path.parent.parent)
30+
module_name = str(relative_path)[:-3].replace(os.sep, ".") # Remove .py extension
31+
modules.append(module_name)
32+
33+
return sorted(modules)
34+
35+
def test_pure_python_import(module_name, base_path):
36+
"""Test if a module can be imported with pure Python (no Django setup)."""
37+
test_script = f'''
38+
import sys
39+
sys.path.insert(0, "{base_path}")
40+
41+
try:
42+
import {module_name}
43+
print("SUCCESS")
44+
except Exception as e:
45+
print(f"ERROR: {{e}}")
46+
'''
47+
48+
# Run in a clean environment without Django setup
49+
env = os.environ.copy()
50+
if 'DJANGO_SETTINGS_MODULE' in env:
51+
del env['DJANGO_SETTINGS_MODULE']
52+
53+
try:
54+
result = subprocess.run(
55+
['python', '-c', test_script],
56+
capture_output=True,
57+
text=True,
58+
env=env,
59+
timeout=30
60+
)
61+
62+
if result.returncode == 0 and "SUCCESS" in result.stdout:
63+
return True, None
64+
else:
65+
error_msg = result.stderr.strip() or result.stdout.strip()
66+
return False, error_msg
67+
except subprocess.TimeoutExpired:
68+
return False, "Import test timed out"
69+
except Exception as e:
70+
return False, f"Failed to run test: {e}"
71+
72+
def main():
73+
"""Main function to test all modules in ansible_base/lib."""
74+
script_dir = Path(__file__).parent
75+
lib_path = script_dir / "ansible_base" / "lib"
76+
77+
if not lib_path.exists():
78+
print(f"ERROR: {lib_path} does not exist")
79+
sys.exit(1)
80+
81+
print("Testing pure Python import compatibility for ansible_base/lib modules...")
82+
print("=" * 70)
83+
84+
modules = find_python_modules(lib_path)
85+
86+
# Modules that are allowed to fail because they inherently require Django
87+
allowed_failures = {
88+
# Abstract models inherently require Django to be initialized
89+
'ansible_base.lib.abstract_models',
90+
'ansible_base.lib.abstract_models.common',
91+
'ansible_base.lib.abstract_models.immutable',
92+
'ansible_base.lib.abstract_models.organization',
93+
'ansible_base.lib.abstract_models.team',
94+
'ansible_base.lib.abstract_models.user',
95+
# View classes require Django REST framework
96+
'ansible_base.lib.utils.views.ansible_base',
97+
'ansible_base.lib.utils.views.django_app_api',
98+
'ansible_base.lib.utils.views.permissions',
99+
'ansible_base.lib.utils.views.urls',
100+
# Router classes require Django REST framework
101+
'ansible_base.lib.routers',
102+
'ansible_base.lib.routers.association_resource_router',
103+
# These modules require Django settings to be configured at import time
104+
'ansible_base.lib.backends.prefixed_user_auth',
105+
'ansible_base.lib.dynamic_config.dynamic_urls',
106+
'ansible_base.lib.serializers.common',
107+
'ansible_base.lib.testing.fixtures',
108+
'ansible_base.lib.testing.util',
109+
'ansible_base.lib.utils.auth',
110+
}
111+
112+
failed_modules = []
113+
passed_modules = []
114+
expected_failures = []
115+
116+
for module_name in modules:
117+
print(f"Testing {module_name}... ", end="", flush=True)
118+
success, error = test_pure_python_import(module_name, str(script_dir))
119+
120+
if success:
121+
print("✓ PASS")
122+
passed_modules.append(module_name)
123+
else:
124+
if module_name in allowed_failures:
125+
print("✗ EXPECTED FAIL")
126+
expected_failures.append((module_name, error))
127+
else:
128+
print("✗ UNEXPECTED FAIL")
129+
print(f" Error: {error}")
130+
failed_modules.append((module_name, error))
131+
132+
print("\n" + "=" * 70)
133+
print(f"Results: {len(passed_modules)} passed, {len(failed_modules)} unexpected failures, {len(expected_failures)} expected failures")
134+
135+
if failed_modules:
136+
print("\nUnexpected failures:")
137+
for module_name, error in failed_modules:
138+
print(f" - {module_name}: {error}")
139+
140+
print("\nThese modules should be importable with pure Python.")
141+
print("Consider moving Django-specific imports inline within functions.")
142+
sys.exit(1)
143+
else:
144+
print("\nAll importable modules passed! ✓")
145+
if expected_failures:
146+
print(f"({len(expected_failures)} modules have expected failures due to Django dependencies)")
147+
sys.exit(0)
148+
149+
if __name__ == "__main__":
150+
main()

0 commit comments

Comments
 (0)