Repository navigation
feat(mixins): remove listener management from MixinEventBus [OMN-1747] - #456
Conversation
Remove listener/consumer lifecycle code from omnibase_core to enforce the architectural invariant: "infra owns Kafka plumbing, core provides contracts + publish-only abstractions." ## Removed Files - mixin_event_listener.py (100% listener code) - model_event_bus_listener_handle.py (listener handle model) - protocol_event_bus_listener.py (orphaned protocol) - test_mixin_event_listener.py (30 tests) - test_model_event_bus_listener_handle.py (62 tests) ## Refactored - mixin_event_bus.py: 40% reduction (1977→1200 lines) - Removed: start_event_listener, stop_event_listener, _event_listener_loop - Removed: get_event_patterns, get_completion_event_type, bind_contract_path - Removed: _create_event_handler, _event_to_input_state, _get_input_state_class - Kept: publish_event, publish_completion_event, apublish_completion_event - Kept: bind_event_bus, bind_registry, bind_node_name ## Updated - model_service_effect.py: Removed stop_event_listener() fallback - test_mixin_event_bus.py: 25 listener tests removed, 53 publish tests kept - conftest.py: Removed dead listener thread cleanup code - mixin_tool_execution.py: Updated docstrings - Various __init__.py: Updated exports ## Added - scripts/validation/validate-no-listener-apis.py: CI guard - .pre-commit-config.yaml: Hook to prevent listener reintroduction Breaking change: MixinEventListener and related APIs removed. Use EventBusSubcontractWiring in omnibase_infra for subscriptions.
📝 WalkthroughWalkthroughThis PR removes event-listener functionality, converting the event bus to publish-only; deletes listener-related classes and tests; adds a pre-commit validation hook and script preventing reintroduction of listener APIs; and bumps version to 0.10.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Git as Git
participant Hook as Pre-commit Hook (ONEX Validation)
participant Script as validate-no-listener-apis.py
participant FS as Repository Files
Dev->>Git: git commit
Git->>Hook: run configured hooks
Hook->>Script: invoke via poetry
Script->>FS: scan `src/omnibase_core` python files
alt forbidden patterns found
Script->>Hook: return non-zero / report violations
Hook->>Git: block commit (fail)
Git->>Dev: show validation failures
else no violations
Script->>Hook: return success
Hook->>Git: allow commit to proceed
Git->>Dev: commit succeeds
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Breaking change release for listener management removal.
4fb1a0c to
52720bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/omnibase_core/mixins/mixin_event_bus.py (1)
1064-1121:⚠️ Potential issue | 🟠 Majordispose_event_bus_resources can raise when runtime state was never bound.
_event_bus_runtime_stateraisesModelOnexErrorif the state is missing, which breaks the method’s “idempotent and safe to call multiple times” contract for unbound instances. Consider using_ensure_runtime_state()(or guarding on state presence) to avoid raising during cleanup.🔧 Suggested fix
- try: - self._event_bus_runtime_state.reset() - except (AttributeError, RuntimeError) as e: + try: + self._ensure_runtime_state().reset() + except (ModelOnexError, RuntimeError) as e: cleanup_errors.append(f"Failed to reset runtime state: {e!r}") emit_log_event( LogLevel.ERROR, f"MIXIN_DISPOSE: Failed to reset runtime state: {e!r}", ModelLogData(node_name=self.get_node_name()), )
🧹 Nitpick comments (2)
src/omnibase_core/models/services/model_service_effect.py (1)
115-120: Guarddispose_event_bus_resourceswith a callable check.
hasattronly verifies presence; a non-callable attribute (e.g., overridden toNone) would raise during shutdown. Consider resolving once and checkingcallablefor resilience.Suggested fix
- if hasattr(self, "dispose_event_bus_resources"): - self.dispose_event_bus_resources() + dispose = getattr(self, "dispose_event_bus_resources", None) + if callable(dispose): + dispose()scripts/validation/validate-no-listener-apis.py (1)
2-39: Trim the module docstring to 3–6 lines.It’s currently far longer than the project standard; move the usage/details into argparse help or a README section.
💡 Proposed docstring trim
-""" -ONEX Listener API Prevention Validation. - -OMN-1747: Listener/consumer lifecycle management was removed from omnibase_core. -This pre-commit hook prevents these APIs from being reintroduced. - -For Kafka consumer management, use EventBusSubcontractWiring in omnibase_infra. - -Detected Patterns: -1. def start_event_listener( - Listener start method -2. def stop_event_listener( - Listener stop method -3. def _event_listener_loop( - Internal listener loop -4. class ModelEventBusListenerHandle - Listener handle model -5. from ... import ... ModelEventBusListenerHandle - Imports of listener handle - -Usage: - # Check specific files (pre-commit mode) - poetry run python scripts/validation/validate-no-listener-apis.py <file1> [file2] ... - - # Check entire src/ directory (default) - poetry run python scripts/validation/validate-no-listener-apis.py - - # Check specific directory - poetry run python scripts/validation/validate-no-listener-apis.py src/omnibase_core/ - - To allow listener APIs in specific files (rare cases), add comment: - # listener-api-ok: reason for exception - -Rationale: - - omnibase_core must remain transport-agnostic (ADR-005) - - Listener lifecycle creates tight coupling to Kafka implementation - - Consumer management belongs in omnibase_infra via EventBusSubcontractWiring - - Core should only define protocols, not implementations - -Reference: - - OMN-1747: Remove listener management from omnibase_core - - docs/architecture/adr/ADR-005-core-infra-dependency-boundary.md -""" +""" +ONEX listener API prevention validator. + +OMN-1747 blocks reintroduction of listener lifecycle APIs in omnibase_core. +Use EventBusSubcontractWiring in omnibase_infra for consumer management. +See ADR-005 for the core/infra boundary. +"""As per coding guidelines: Module docstrings must be concise (3-6 lines max for simple modules); follow PEP 257 with module docstring first, then
from __future__ import annotations.
Summary
Remove listener/consumer lifecycle code from
omnibase_coreto enforce the architectural invariant: "infra owns Kafka plumbing, core provides contracts + publish-only abstractions."This resolves the competing subscription systems between:
omnibase_core:MixinEventBuslistener management (REMOVED)omnibase_infra:EventBusSubcontractWiring(KEPT - canonical approach)Changes
Removed Files (5 files, -4,368 lines)
mixin_event_listener.pymodel_event_bus_listener_handle.pyprotocol_event_bus_listener.pytest_mixin_event_listener.pytest_model_event_bus_listener_handle.pyRefactored
mixin_event_bus.py: 40% reduction (1977 → 1200 lines)start_event_listener,stop_event_listener,_event_listener_loopget_event_patterns,get_completion_event_type,bind_contract_path_create_event_handler,_event_to_input_state,_get_input_state_classpublish_event,publish_completion_event,apublish_completion_eventbind_event_bus,bind_registry,bind_node_nameUpdated
model_service_effect.pystop_event_listener()fallbacktest_mixin_event_bus.pyconftest.pymixin_tool_execution.py__init__.pyAdded (CI Guard)
scripts/validation/validate-no-listener-apis.py.pre-commit-config.yamlvalidate-no-listener-apisInventory Evidence
Production references verified:
start_event_listener()model_service_effect.pyonly had shutdown fallback (now removed)Test Plan
Breaking Changes
MixinEventListenerand related APIs removed.Migration: Use
EventBusSubcontractWiringinomnibase_infrafor Kafka consumer lifecycle management.References
Summary by CodeRabbit
Refactor
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.