crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.48k stars 768 forks source link

Unwanted noise #1175

Open oddjobz opened 5 years ago

oddjobz commented 5 years ago

Is there a reason why we need this as a warning?

https://github.com/crossbario/autobahn-python/blob/0b0f3262903eb879a9ab6b313ab1a77e4d525726/autobahn/twisted/component.py#L332

When starting a lot of components, it seems like unnecessary noise, could this at least be made an 'info' or maybe even a 'debug' ?

meejah commented 5 years ago

Yeah, that could probably be a "info" or something instead (or, you could pass in the reactor, which is considered "better practice" really).

oddjobz commented 5 years ago

Mmm .. on the one hand I can see why you would want to put out a warning if people "should" be supplying a reactor .. but on the other hand, if a reactor "should" be specified, maybe accepting a default isn't the right thing to do? (a warning because you've used a valid default seems a bit .. odd ?)

.. do you have an example of using "run()" and passing a reactor, I'm not seeing it on the demos?

meejah commented 5 years ago

react passes the reactor: https://github.com/crossbario/autobahn-python/blob/0b0f3262903eb879a9ab6b313ab1a77e4d525726/autobahn/twisted/component.py#L379

...which then passes it on to start here: https://github.com/crossbario/autobahn-python/blob/0b0f3262903eb879a9ab6b313ab1a77e4d525726/autobahn/wamp/component.py#L931

meejah commented 5 years ago

I think it might be optional because "originally" start took zero arguments? (In any case, it could probably be an info instead of a warning at least...)

oddjobz commented 5 years ago

Sorry, not following .. examples use;

run([components])

Can't seem to find an example of this with a reactor?

meejah commented 5 years ago

Yes, run doesn't take a reactor as it uses react underneath. react (and run) are supposed to be the "high level" interfaces, and handle selecting a reactor, setting up logging, etc. If you want more control, you use start() .. and then you have to select the reactor yourself and start logging yourself.

oddjobz commented 5 years ago

Mmm, so .. if passing a reactor is "better practice" .. that sort of implies that maybe I shouldn't be using run?

oberstet commented 5 years ago

I think what @meejah is trying to get at is:

that being said: Twisted itself sadly doesn't follow that strictly .. asyncio (where it's "loop" rather than "reactor") is better. implicit or ad-hoc reactor imports in Twisted and Twisted based library and app code is .. a PITA sometimes. if you dig through CB/AB history/commits, you'd find a bunch of issues related to that;) it's the kinda of bug as Python2->3 change of string base type to real unicode. if python would have had it done that correctly in the first place, it would have saved the whole community decades of accumulated work to clean up later .. hah. shit happens.

oddjobz commented 5 years ago

Sure .. I find Twisted in general is a lot "nosier" than I would expect. If I'm using an established library, I prefer relative silence unless something unexpected happens, or unless I explicitly ask for more information .. a dozen warnings for calling "run" seemed a little excessive .. :)

meejah commented 5 years ago

Yeah, that quoted text :)

Also, asyncio doesn't get it right since there's get_default_loop() so some asyncio libraries have "pass the loop" throughout them, and some depend on "a global loop, somewhere". So it's kind of the same situation that Twisted is currently in :/

meejah commented 5 years ago

You're getting those warnings just calling "run()"? (Reading the code, I'd expect that you wouldn't get the warning then, because _run() passes in the loop/reactor..?)

oberstet commented 5 years ago

Also, asyncio doesn't get it right since .. so it's kind of the same situation that Twisted is currently in :/

oh, I see! well, so they repeated the same mistake. well. too bad:(

oddjobz commented 5 years ago

Yeah, my startup looks like this;

2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 Using default reactor
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"
2019-04-18T03:11:29+0100 connecting once using transport type "websocket" over endpoint "tcp"