Skip to content

feat: add extension points for DI provider and event handler overrides#90

Merged
rorybyrne merged 3 commits into
mainfrom
89-feat-pluggable-runner-support-via-di-composition-root-overrides
Mar 21, 2026
Merged

feat: add extension points for DI provider and event handler overrides#90
rorybyrne merged 3 commits into
mainfrom
89-feat-pluggable-runner-support-via-di-composition-root-overrides

Conversation

@rorybyrne

Copy link
Copy Markdown
Contributor

Summary

  • create_app() now accepts providers and extra_handlers keyword arguments, allowing infrastructure to be swapped at startup
  • create_container() accepts *extra_providers (appended after defaults, last-wins override) and extra_handlers (merged into EventProvider)
  • EventProvider constructor accepts extra_handlers, merging them with core handlers for subscription routing, WorkerPool registration, and DI resolution
  • Handler DI bindings moved from class-body locals() trick to dynamic self.provide() in __init__ to support runtime extension

Test plan

  • All 804 existing unit tests pass with no changes
  • Smoke-tested provider override and extra handler registration with full DI container

create_app() now accepts `providers` and `extra_handlers` kwargs,
allowing external packages to swap infrastructure adapters (e.g.
container runners, storage backends) and register additional event
handlers without duplicating any app wiring.

- create_container() accepts *extra_providers and extra_handlers
- EventProvider accepts extra_handlers, merging them with core handlers
  for subscription routing, WorkerPool registration, and DI resolution
- Handler DI bindings moved from class-body locals() trick to dynamic
  self.provide() in __init__ to support runtime extension
@rorybyrne rorybyrne linked an issue Mar 21, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented Mar 21, 2026

Copy link
Copy Markdown

Code Coverage

Package Line Rate Complexity Health
. 93% 0
application 100% 0
application.api 100% 0
application.api.rest 82% 0
application.api.v1 82% 0
application.api.v1.routes 64% 0
application.event 100% 0
domain 100% 0
domain.auth 100% 0
domain.auth.command 90% 0
domain.auth.event 100% 0
domain.auth.model 93% 0
domain.auth.port 99% 0
domain.auth.query 93% 0
domain.auth.service 91% 0
domain.auth.util 100% 0
domain.auth.util.di 81% 0
domain.curation 100% 0
domain.curation.adapter 100% 0
domain.curation.command 100% 0
domain.curation.event 100% 0
domain.curation.handler 92% 0
domain.curation.model 100% 0
domain.curation.port 100% 0
domain.curation.query 100% 0
domain.curation.service 100% 0
domain.deposition 100% 0
domain.deposition.adapter 100% 0
domain.deposition.command 87% 0
domain.deposition.event 100% 0
domain.deposition.handler 100% 0
domain.deposition.model 98% 0
domain.deposition.port 100% 0
domain.deposition.query 86% 0
domain.deposition.service 97% 0
domain.deposition.util.di 94% 0
domain.discovery 100% 0
domain.discovery.model 100% 0
domain.discovery.port 100% 0
domain.discovery.query 100% 0
domain.discovery.service 94% 0
domain.discovery.util 100% 0
domain.discovery.util.di 94% 0
domain.export 100% 0
domain.export.adapter 100% 0
domain.export.command 100% 0
domain.export.event 100% 0
domain.export.model 100% 0
domain.export.port 100% 0
domain.export.query 100% 0
domain.export.service 100% 0
domain.feature 100% 0
domain.feature.event 100% 0
domain.feature.handler 100% 0
domain.feature.model 0% 0
domain.feature.port 100% 0
domain.feature.service 96% 0
domain.feature.util 100% 0
domain.feature.util.di 100% 0
domain.index 100% 0
domain.index.event 100% 0
domain.index.handler 75% 0
domain.index.model 84% 0
domain.index.service 100% 0
domain.record 100% 0
domain.record.adapter 100% 0
domain.record.command 100% 0
domain.record.event 100% 0
domain.record.handler 100% 0
domain.record.model 100% 0
domain.record.port 100% 0
domain.record.query 100% 0
domain.record.service 90% 0
domain.search 100% 0
domain.search.adapter 100% 0
domain.search.command 100% 0
domain.search.event 100% 0
domain.search.model 100% 0
domain.search.port 100% 0
domain.search.query 100% 0
domain.search.service 100% 0
domain.semantics 100% 0
domain.semantics.command 94% 0
domain.semantics.event 100% 0
domain.semantics.handler 100% 0
domain.semantics.model 100% 0
domain.semantics.port 100% 0
domain.semantics.query 90% 0
domain.semantics.service 100% 0
domain.semantics.util 100% 0
domain.semantics.util.di 93% 0
domain.shared 94% 0
domain.shared.authorization 87% 0
domain.shared.model 93% 0
domain.shared.port 100% 0
domain.source 100% 0
domain.source.event 100% 0
domain.source.handler 92% 0
domain.source.model 100% 0
domain.source.port 100% 0
domain.source.schedule 67% 0
domain.source.service 98% 0
domain.validation 100% 0
domain.validation.adapter 100% 0
domain.validation.command 0% 0
domain.validation.event 100% 0
domain.validation.handler 90% 0
domain.validation.model 85% 0
domain.validation.port 100% 0
domain.validation.query 100% 0
domain.validation.service 89% 0
domain.validation.util.di 92% 0
infrastructure 100% 0
infrastructure.auth 56% 0
infrastructure.event 79% 0
infrastructure.http 92% 0
infrastructure.index 82% 0
infrastructure.index.vector 73% 0
infrastructure.messaging 100% 0
infrastructure.oci 54% 0
infrastructure.persistence 80% 0
infrastructure.persistence.adapter 59% 0
infrastructure.persistence.mappers 56% 0
infrastructure.persistence.repository 32% 0
infrastructure.source 91% 0
sdk 100% 0
sdk.index 100% 0
util 100% 0
util.di 62% 0
Summary 80% (5423 / 6799) 0

@greptile-apps

greptile-apps Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds extension points to create_app() and create_container() so external hosts can inject custom Dishka providers and extra event handlers without forking application wiring. The EventProvider is refactored away from a class-body locals() trick to a cleaner self.provide() loop in __init__, which is the correct approach for runtime registration. A derive_frontend_url config validator is also bundled in.

Key changes:

  • create_app(providers=..., extra_handlers=...) and create_container(*extra_providers, extra_handlers=...) are cleanly designed; last-wins provider semantics are well-documented.
  • EventProvider.__init__ correctly builds _all_handlers by concatenating _CORE_HANDLERS with any extras and registers all of them via self.provide().
  • server/osa/config.py — the new derive_frontend_url model validator uses the default URL string as a sentinel, which is ambiguous: an explicit OSA_FRONTEND__URL set to the same default string is silently overridden. It also changes the local-dev default by stripping the :3000 port when domain=localhost, which can break CORS allow-lists and redirect targets.
  • server/osa/infrastructure/event/di.py — no deduplication guard means passing a core handler in extra_handlers would call self.provide() twice for the same type, potentially causing a startup error.

Confidence Score: 3/5

  • Safe to merge for the DI extension-point changes; the bundled config validator introduces a concrete behavioural regression for local development that should be resolved first.
  • The DI provider and event-handler extension points are well-implemented and the 804 passing tests validate the core changes. However, the derive_frontend_url validator in config.py changes the effective default of frontend.url for local development (drops the :3000 port) and can silently discard an explicit env-var override — a concrete bug that could break CORS or OAuth redirects in any environment using the default domain. That single issue in an unrelated file is enough to withhold a 4.
  • server/osa/config.py — the derive_frontend_url validator logic needs to be revisited before merging.

Important Files Changed

Filename Overview
server/osa/infrastructure/event/di.py Replaces the class-body locals() trick for handler DI bindings with dynamic self.provide() calls in __init__, and exposes extra_handlers to allow runtime handler extension. The core logic is sound; no guard exists against accidentally passing a core handler in extra_handlers, which would cause double registration.
server/osa/application/di.py Cleanly adds *extra_providers (varargs) and extra_handlers parameters to create_container(). Extra providers are appended after built-in ones for last-wins override semantics. No issues found.
server/osa/application/api/rest/app.py Adds keyword-only providers and extra_handlers to create_app() and threads them through to create_container(). Module-level app = create_app() is unchanged and works correctly with no arguments.
server/osa/config.py Adds derive_frontend_url model validator that auto-derives frontend.url from base_url when the field holds the default string value. This sentinel approach is ambiguous — an explicit env-var set to the default string is silently overridden — and changes the local-dev default (removes the :3000 port), potentially breaking CORS and redirect configuration.

Sequence Diagram

sequenceDiagram
    participant Host as External Host
    participant CA as create_app()
    participant CC as create_container()
    participant EP as EventProvider.__init__()
    participant DK as Dishka Container

    Host->>CA: create_app(providers=[K8sProvider()],<br/>extra_handlers=[MeterUsage])
    CA->>CC: create_container(*providers,<br/>extra_handlers=extra_handlers)
    CC->>EP: EventProvider(extra_handlers=[MeterUsage])
    EP->>EP: _all_handlers = [*_CORE_HANDLERS, MeterUsage]
    loop for each handler in _all_handlers
        EP->>EP: self.provide(handler_type, scope=UOW)
    end
    EP-->>CC: EventProvider instance
    CC->>DK: make_async_container(..., *extra_providers)
    DK-->>CC: AsyncContainer
    CC-->>CA: AsyncContainer
    CA-->>Host: FastAPI app
Loading

Last reviewed commit: "feat: add extension ..."

Comment on lines +85 to +91
self._all_handlers = HandlerTypes([*_CORE_HANDLERS, *(extra_handlers or [])])

# Register DI bindings for every handler (core + extra).
# Each handler becomes a UOW-scoped dependency that Dishka can
# instantiate with its declared fields injected.
for handler_type in self._all_handlers:
self.provide(handler_type, scope=Scope.UOW)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No guard against duplicate handler registration

_CORE_HANDLERS and extra_handlers are concatenated without deduplication. If a caller passes a handler that already exists in _CORE_HANDLERS, self.provide() will be called twice for the same concrete type. Depending on the Dishka version this will either silently ignore the second registration or raise a DuplicateFactoryError at container start-up.

Adding a deduplication step (or at least a clear error) keeps the failure surface small:

seen: set[type] = set()
for handler_type in self._all_handlers:
    if handler_type in seen:
        raise ValueError(
            f"Duplicate event handler registration: {handler_type.__name__!r}. "
            "Remove it from extra_handlers — it is already registered as a core handler."
        )
    seen.add(handler_type)
    self.provide(handler_type, scope=Scope.UOW)

Comment thread server/osa/config.py
- Remove module-level app = create_app() to eliminate import side effects
- Switch uvicorn to --factory mode in Dockerfile and Justfile
- Add test_app_factory: tests create_app and create_container with
  provider overrides, extra event handlers, and both combined
- Add test_event_provider: tests EventProvider default behaviour, extra
  handler merging, subscription registry, and DI resolution
Raise ValueError if the same handler type appears in both core and
extra handlers, or is passed twice in extra_handlers. Prevents silent
misbehaviour or DuplicateFactoryError depending on Dishka version.
@rorybyrne rorybyrne merged commit 0387bdd into main Mar 21, 2026
9 checks passed
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.

feat: pluggable runner support via DI composition root overrides

1 participant