Code Monkey home page Code Monkey logo

Comments (7)

nbraun-wolf avatar nbraun-wolf commented on June 14, 2024 1

Yes kind of, putting it inside create_app is only half the solution since then how would I import it somewhere else and use it? But you understand the core of the problem. Somewhere else someone has just helped me to figure out how to do it.

In create_app the I create the client as you suggest but as global. And then in any other file that needs, it I can import main and then use main.mqtt.

# in main.py
def create_app():
    global mqtt
    mqtt = Client("localhost")
   ...

# in router.py
import main
@router.get("/")
async def index():
    await main.mqtt.publish("test", "hi")
    return {"message", "published"}
```

from aiomqtt.

frederikaalund avatar frederikaalund commented on June 14, 2024

Thanks for opening this issue. Let me have a look. :)

The app runs fine, and the client seems to close cleanly.

Glad to hear that it works in the app.

However, when I run my pytests I get an error after the first test

Seems like you have a good test setup with proper event loop isolation. πŸ‘ Let's continue in that line with asyncio-mqtt as well: A new Client instance for each test.

But it breaks when trying to use the same client ID for the subscriber and the publisher clients

Two clients with the same ID is a user error. Make sure to use unique ID for each client. You can use the client_id keyword argument to reuse existing IDs, but don't use two identical IDs at once. The broker won't be able to distinguish between two clients if they use the same client ID.

Conceptionally I have one instance of app, so it should have only a single client ID and get treated by the broker as one entity, which doesn't seem to be possible.

Use a single Client instance for each client ID. Once you disconnect a client (Client.disconnect or Client.__aexit__) the Client instance is used up but the client ID still remains. If you want to reuse the client ID, create a new Client instance with the client_id keyword argument.

Apart from the disconnection problem, I feel like It's wasteful to create constantly new clients and making a new connection first before being able to publish a message.

For the tests, I wouldn't worry at all. After all, how often do you run these tests? One connection per test sounds fine to me. Moreover, it ensures proper test isolation. I.e., the behaviour of test A can't affect test B because they each have their own client. That's a good thing. :)

For the application, it's a real concern. Ideally, we create a single Client during startup and use that throughout the application's life time for both publish and subscribe. From what I gather, you already do this (good job!) and it works in your app.

Does it make sense?

from aiomqtt.

nbraun-wolf avatar nbraun-wolf commented on June 14, 2024

Thanks for the answer. It makes sense, but I still don't know how I can have both, a new client for each test and a single client for the application when it really runs. Since the application is initializing the client and the tests initialize the app. I can only have one of the two things, it seems.

  • I can create a new client for each publish and subscribe in the app, which results in pytests not failing but having a strange application architecture.
  • Or I can create a single client in the app, which results in the pytests failing.

The only kind of workaround I could come up with is to use a factory pattern for each file that relies on the client. Which can be seen in this branch https://github.com/nbraun-wolf/async-mqtt-fastapi-pytest/tree/closure. With this, am a not sure if this is a nice way to do it. It makes everything nested and a bit cumbersome, but at least it results in a fresh client every time the app factory is called, since it's not imported as a global side effect.

As an example, the router would then look like the below. And the main app calls create_router to use it.

def create_router(mqtt):

    from fastapi import APIRouter

    router = APIRouter()

    @router.get("/")
    async def index():
        await mqtt.publish("test", "hi")
        return {"message", "published"}

    return router

from aiomqtt.

frederikaalund avatar frederikaalund commented on June 14, 2024

What about a new application per test? I mean, if you already have a new event loop per test, you should also have a new application per test.

from aiomqtt.

nbraun-wolf avatar nbraun-wolf commented on June 14, 2024

That's what I am doing. But the client still kind of knows about the previous event loop. Please take a look at this repo once, which is a simplified example in itself. https://github.com/nbraun-wolf/async-mqtt-fastapi-pytest

My pytest fixture looks like this.

@pytest.fixture
async def http_client():
    app = create_app()
    async with AsyncClient(
        app=app,
        base_url="http://pytest",
    ) as client, LifespanManager(app):
        yield client

And yet the client throws that error, likely because it is imported as a global side effect more or less.

def create_app():
    from client import mqtt
    from router import router  # depends on mqtt
   ...
   return app

Since the router in this case depends on the client. I need to have a way to imported there. Hence, it sits in its own module. While I still only call connect in the app factory as you see in my initial code snippet of this issue.

This doesn't seem to be enough. I could only stop it from happening with this kind of closure approach I have shown in the previous message.

from aiomqtt.

frederikaalund avatar frederikaalund commented on June 14, 2024

That's what I am doing. But the client still kind of knows about the previous event loop.

The asyncio_mqtt.Client gets the event loop during __init__. Each Client instance knows nothing about the previous instances. There is complete isolation.

It is the responsibility of the user (you) to ensure that the event loop keeps running for the lifetime of the Client instance. We (the asyncio-mqtt maintainers) can't help you there. :)

I'm sure that it's possible to make pytest behave accordingly. πŸ‘ That, however, is out of the scope of asyncio-mqtt. You must seek support for such things elsewhere. E.g., on StackOverflow.


All that being said, I took a look at your repo anyhow. In https://github.com/nbraun-wolf/async-mqtt-fastapi-pytest/blob/main/client.py you create a global client:

from asyncio_mqtt import Client

mqtt = Client("localhost")

This is bad. Python runs this code once on the first import. All subsequent imports get the same client instance! This is most likely what causes your issues. :)

Just remove main/client.py and put those two lines inside create_app. That should fix things. πŸ‘

from aiomqtt.

frederikaalund avatar frederikaalund commented on June 14, 2024

Yes kind of, putting it inside create_app is only half the solution since then how would I import it somewhere else and use it?

Don't. :) It's an anti-pattern.

from aiomqtt.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.