Skip to content

Register extension packages before executing their initializers - #203

Open
sylvesterkaczmarek wants to merge 1 commit into
nasa:mainfrom
sylvesterkaczmarek:202-fix-extension-package-initialization
Open

sylvesterkaczmarek wants to merge 1 commit into
nasa:mainfrom
sylvesterkaczmarek:202-fix-extension-package-initialization

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Sep 26, 2026 •

Copy link
Copy Markdown

Fixes #202.

Changes

Register each external plugin or service package in sys.modules before executing its initializer, so relative imports and self-imports work when the extension directory is outside sys.path. If initialization raises, remove the failing package's cache entry and propagate the original exception. Existing cached packages continue to be reused.

Both public importer functions retain their existing naming, instantiation and configuration behavior. Legacy plugin paths ending in __init__.py still work. Successfully imported child modules are not removed as part of the failing package cleanup, matching normal Python import behavior.

Validation

Add 15 cases using actual temporary packages, rather than mocked import machinery. They cover relative imports, module identity, failure cleanup and retry for RuntimeError/SystemExit/KeyboardInterrupt, cached packages, missing initializers, and the legacy plugin path.

All 371 existing tests pass on the unchanged baseline. All 386 tests pass with the fix on macOS arm64/Python 3.12.11, using randomized test orders with seeds 42 and 7. Both changed importers have 100% line and branch coverage. Restoring only the two original importer files produces 11 failures and four passing controls in the new tests; restoring the submitted files passes all 386 tests again.

Exact-commit hosted validation passes on Ubuntu 24.04 with Python 3.8, 3.10 and 3.12, testing f5c0ee026ce792fabe8fadf201704c9dd526c4a1. Each job runs the full suite, checks importer coverage, runs repository-wide formatting and the existing advisory pylint configuration, reproduces the original failures, restores the submitted sources and reruns the full suite. Pylint retains its existing advisory warnings. Logs are retained as artifacts. NASA's upstream workflows currently require approval before running; the linked results are from the fork.

Scope

Three files changed: the two importers and the new regression tests. No dependency, configuration, workflow or deployment changes are included. The validation workflow is on a separate fork branch. No live SBN, Redis or cFS services were used; existing service mocks remain unchanged.

Contribution status

Draft pending confirmation of the Individual Contributor License Agreement required by the repository's contributing instructions. This PR does not assert that the agreement has been completed or accepted.

Fixes nasa#202. Make relative imports available during package initialization and remove the failed package cache entry when an initializer raises. Cover plugin and service loading with real temporary packages.
@sylvesterkaczmarek
sylvesterkaczmarek marked this pull request as ready for review September 26, 2026 15:44
@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Checking back on the extension registration fix. Is anything else needed from me?

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (16fe993) to head (f5c0ee0).
⚠️ Report is 163 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #203    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           26        31     +5     
  Lines          970      1178   +208     
  Branches       136       168    +32     
==========================================
+ Hits           970      1178   +208     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External plugin and service packages cannot use relative imports in their initializers

2 participants