FastAPI sync endpoints & sync/async co-use

[tuukkamustonen]

The docs instruct to use create_async_container but it won't work with sync endpoints.

You'll end up awaiting for sync function in https://github.com/reagento/dishka/blob/develop/src/dishka/integrations/base.py#L207

The lib hard-codes is_async=True in https://github.com/reagento/dishka/blob/develop/src/dishka/integrations/fastapi.py#L47

Are sync endpoints simply not supported?

In general, how can I use same container in both sync and async contexts? Consider creating an async container for FastAPI, but then tests are written as sync, and you want to access the same container from there - but can't call async AsyncContainer.get from sync method.

[Tishka17]

I guess we can use Depends for sync handlers. But anyway we need somehow distinguish sync/async containers

[tuukkamustonen]

Could you elaborate what you mean by that - what's the plan?

You can call sync resources from async context (though if there's any IO, shouldn't without asyncio.to_thread()) but you can't call async resources from sync contexts (except maybe via run_until_complete()).

As a reference, https://github.com/hynek/svcs doesn't separate containers per sync/async, and provides get() for sync calls, aget() for async. Would something similar make sense?

[Tishka17]

We can definitly provide usage of sync container in any endpoints - we just call .get and that's it.

But calls to async container should be done only within same loop only. Fastapi (Starlette?) runs all sync handlers in separate threads, so they do not have access to main loop. We can use Depends() as a workaround here, though it make things slower. Is there any builtin way to run sync funcion as fastapi does itself?

Having get and aget methods together could be a good idea, but not sure. Anyway it breaks current API and I want to keep compatibility for a long time. =)

[tuukkamustonen]

We can definitly provide usage of sync container in any endpoints - we just call .get and that's it.

But if you create_async_container for the app, there's no sync container around.

What would be needed is sync interface for the async container (which can hold also sync factories and resources), right? Like sget for sync get 🤔

But calls to async container should be done only within same loop only. Fastapi (Starlette?) runs all sync handlers in separate threads, so they do not have access to main loop. We can use Depends() as a workaround here, though it make things slower.

IIRC it's possible to access the main event loop from threads (but it requires some boilerplate).

Anyway, there's no need to access async resources from threads. So just having the async/sync container around, and having the interface to access the sync resources only, wouldn't that suffice?

Is there any builtin way to run sync funcion as fastapi does itself?

I think it uses AnyIO to launch those threads, if that's what you mean. Not sure, don't know the details.

Having get and aget methods together could be a good idea, but not sure. Anyway it breaks current API and I want to keep compatibility for a long time. =)

Yeah... maybe though:

• Sync interface would need to be added to AsyncContainer, to acquire sync resources in sync contexts
• Docs should be clarified - if you need both async and sync access, are you supposed to create two containers, and maybe somehow link those together? Apparently not, and that sync interface would be needed.

[Tishka17]

We cannot have sync interface for some part of async container as potentially any resource can require async initialization on some step. It depends on providers combination and can be overloaded. Changing this will require even more explicit and complicated api due to additional validations. I have some unclear ideas on container interface validations we can return to that question together with them.

[tuukkamustonen]

We cannot have sync interface for some part of async container as potentially any resource can require async initialization on some step.

Could accept that and explicitly raise if that happens? When you attempt to retrieve a sync resource from a sync context, you most likely don't (want to) depend on async resources.

At least in this case, I would be asking for sync resources especially because they're needed in a thread that was spawned, because the code didn't support async.

(Alternatively, it could run_until_complete() for async dependencies though that could get complicated/hacky, not sure.)

But yeah, I'm lacking understanding here and just throwing perspective/ideas. You know the library, not me 😄 Thanks for your effort already 🙏🏻

Do you think you'll be resolving / adding support for something like this in the near future? I'm evaluating the lib and it works beautifully except maybe for this. There's a workaround - converting all FastAPI endpoints/code async, but that would take some effort, and kinda fights against the idea that you run both async and sync endpoints.

[Tishka17]

When you attempt to retrieve a sync resource from a sync context, you most likely don't (want to) depend on async resources.

In this case you do not use Async container. From my perspective mixing async/sync handlers in the same fastapi application is not a good idea. But, the real problem is that I cannot yet see a proper solution for a request. We need to think more, to try different approaches, though I do not see any priority on this feature.

It would be great to see prototypes here in comments.

[tuukkamustonen]

In this case you do not use Async container

But you need that async container, for the async endpoints. Yeah, if you only used one or the other, you wouldn't have the problem.

The reality, though, is that people do have applications with both sync and async endpoints. Traditionally, we built only sync, and most likely there are bunch of projects out there still in the (maybe forever) transformation, doing both async and sync, because it's supported.

(I agree it's painful to mix those, but that's what we have now.)

The other use case where async and sync are often mixed, is testing out an async Starlette/FastAPI/any-web-framework app with sync tests. If in that scenario you want to grab anything from the container, now in a sync test you cannot (unless maybe via run_until_complete() and that might work just fine, need to test).

I'll post here if I come up with something.

[tuukkamustonen]

Hey, I tested out locally extending the AsyncContainer, and adding sync interface to it. Roughly copy-pasting the applicable parts and adjusting as needed. Something like:

class HybridContainer(AsyncContainer):

    def __init__(
        self,
        ...
        sync_lock_factory: Callable[[], AbstractContextManager[Any]] | None = None,
    ):
        super().__init__(...)
        self.sync_lock: AbstractContextManager[Any] | None
        if sync_lock_factory:
            self.sync_lock = sync_lock_factory()
        else:
            self.sync_lock = None

    def sget(self, dependency_type: Any, component: Component | None = DEFAULT_COMPONENT) -> Any:
        sync_lock = self.sync_lock
        key = DependencyKey(dependency_type, component)
        if not sync_lock:
            return self._sync_get_unlocked(key)
        with sync_lock:
            return self._sync_get_unlocked(key)

    def _sync_get_unlocked(self, key: DependencyKey) -> Any:
        ...point to sget instead...

    def __call__(...) -> AsyncContextWrapper:
        ...point to HybridContainer instead...

Then make_hybrid_container adding the sync_lock_factory and simply pointing to the new the HybridContainer type.

(This is not the interface I'm proposing, but an example.)

Also added some small bits to #382 and I was able to make the sync endpoints callable, and injection working to the classes there.

I don't know the Dishka code or design, really, so what would be the problems to solve in this approach (ie. providing separate interfaces on the same container)? Something with locking? Something with thread safety? None?

You mentioned earlier:

We cannot have sync interface for some part of async container as potentially any resource can require async initialization on some step.

I would just accept that - if you need async resources, use the async interface. If you need sync resources, use the sync interface. No problem carrying out both dependencies within the same container, though (already possible, as you can define sync resources in async container, and grab those as usual, just calling the async get but you cannot do the opposite, and still wouldn't).

[Tishka17]

That looks really interesting. I am still not sure we want this integrated into library, but I am glad that dishka allows such hacks without huge modifications on its core.
Please, share your experience if you will continue going that way

[tuukkamustonen]

Hey, sorry for delay in the reply.

If you're not interested in integrating that functionality into the library, I'll probably have to go with some other lib. I don't want to be hacking in too much features locally.

If your stance is that sync/async co-use is not supported by the library, maybe you could highlight that somewhere in the docs, and add to the comparison matrix - it can be a big decision factor for some (lots of legacy codebases out there migrating gradually from sync to async).

That said, I do hope you'll consider it and end up thinking it's a sensible thing to have, with little added complexity 🙏🏻

[Tishka17]

Ok, I'll express my opinion here

1. I/O in IoC container is probably not a good thing to be used often, but we allow it. It's up to use it or not
2. If you allow async I/O in container you cannot know if some object is created in a sync way or not. It is a matter of encapsulation: you request object and you do not worry how it is created. Even if it doesn't have async methods it can require an object which requires another object which on some part of initialization or finalization requires access to an asyncio loop.
3. If you are sure that you have some sync subset and some async subset of object you can just use 2 containers.
4. Creating own intergrations is expected way to use library.

So, one more possible solution:

1. Create 2 sets of providers: sync and async ones.
2. Create 2 containers: make_container(*sync_providers), make_async_container(*sync_providers, *async_providers)
3. Setup builtin fastapi integration for async container
4. Copy paste integration code to implement sync version of it. It should use different key for storing its container as they need to exist.

By doing this way you will have 2 setup_dishka calls, and 2 versions of inject - for sync and async.
Downside is that by you cannot share dependencies between two containers, so you should no use them in middlewares.

Anyway, it's up to you what to use. I do not see good solutions here, that is why I am sceptic about including all this stuff in library.

[tuukkamustonen]

Hey thanks for writing that out.

1. I/O in IoC container is probably not a good thing to be used often, but we allow it. It's up to use it or not

Yeah construction of those resources shouldn't probably do IO, though they can. Responsibility of the developer, I guess. But is this related to something I wrote earlier, or a generic thought?

2. If you allow async I/O in container you cannot know if some object is created in a sync way or not. It is a matter of encapsulation: you request object and you do not worry how it is created. Even if it doesn't have async methods it can require an object which requires another object which on some part of initialization or finalization requires access to an asyncio loop.

Yes, I think that's fine (error would be risen). I guess our views just differ here, doesn't feel like there would be a lack of understanding.

If we think is it good API design that (sync call of) get(resource_that_has_async_dependency) might or might raise, if it ends up requesting an async resource, depending on if it was already initialized and cached or not... well yeah I agree with you that it's certainly not optimal and it can be a weapon of mass destruction for some 😄

4. Creating own intergrations is expected way to use library.

Sure, but there I think you should match the expectations that the integrated lib provides (e.g. sync methods for FastAPI, because FastAPI supports those). Supporting FromDishka in sync callables should be there.

(Injection into classes as in #382 is another thing, I understand if it's out of scope.)

---

Regarding sync + async containers:

Creating separate containers adds some bloat, because you might use a sync resource as a dependency for both async and sync resources. Having two containers requires initializing and storing it twice (CPU+RAM), and if you want to mock/manipulate it, you again need to do it twice 🤔

Though, I'm not sure much of those we would have. Maybe it's not a real problem, or at least not a serious problem. Somewhat annoying, yes, but I can try experiment with that, too.