aminghadersohi commented on code in PR #44504:
URL: https://github.com/apache/superset/pull/44504#discussion_r4070044374


##########
superset/security/password_change.py:
##########
@@ -36,14 +36,15 @@
 from sqlalchemy.exc import IntegrityError
 
 from superset.utils.decorators import transaction
+from superset.views.health import health_blueprint

Review Comment:
   Confirmed and fixed in b7be7b44df: moved only the health_blueprint import 
into register_password_change_enforcement.
   
   Concrete evidence: with PYTHONPATH="$PWD:$PWD/superset-core/src", a 
fresh-process import of superset.security.password_change before this change 
traversed views/__init__.py → alerts → base → deletion_retention/window → 
key_value → models.core, then failed in encrypted_field_factory.create with 
"App not initialized yet. Please call init_app first". After the change, the 
same import succeeds and superset.views is absent from sys.modules. This 
establishes a real premature-initialization dependency; I am not claiming an 
observed circular-import failure or a measured full-app startup speedup.
   
   Validation:
   - PYTHONPATH="$PWD:$PWD/superset-core/src" pytest -q 
tests/unit_tests/security/test_password_change.py 
tests/unit_tests/security/test_password_change_session_invalidation.py 
tests/unit_tests/initialization_test.py tests/unit_tests/views/test_health.py: 
95 passed in 4.36s, no test failures.
   - The health regression initializes the real Superset app, checks GET and 
HEAD on every registered health-blueprint rule for a flagged authenticated 
session, verifies unchanged 200 responses and no attribute lookup, and requires 
/ to redirect to /resetmypassword/form as a positive control that enforcement 
actually registered and ran.
   - pre-commit run on the explicitly staged file, pre-commit run --files on 
all three PR paths, and pre-commit run mypy: all applicable hooks passed.
   
   The initial import probe without the local superset-core/src path failed 
with ModuleNotFoundError: superset_core; the corrected commands above use the 
repository package. The localhost health check found no running server; no dev 
environment was started. Pushed normally to fork after confirming the PR head 
ref matches the local branch. No reviewer re-request.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to