Opened 16 years ago

Closed 16 years ago

#1364 enhancement closed invalid (invalid)

[PATCH] re-installation of the same reactor

Reported by: antony Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: itamarst, antony Branch:
Author:

Description


Attachments (1)

main.diff (685 bytes) - added by antony 16 years ago.

Download all attachments as: .zip

Change History (10)

Changed 16 years ago by antony

Attachment: main.diff added

comment:1 Changed 16 years ago by antony

I am using two independent modules in my application that require wxreactor, so
they both install it. The second time causes an AssertionError which I simply
ignore, but it seems to make sense that if it's the same reactor, no exception
will be raised. Patch attached.

comment:2 Changed 16 years ago by itamarst

You probably don't want to intall the reactor in two different modules though.
Reactor installation is typically the job of the control script (e.g. twistd),
whichever piece of code eventually calls reactor.run(). Maybe someone will want
to use parts of your package with a different reactor; you shouldn't have
library code installing the reactor.

comment:3 Changed 16 years ago by antony

Well, I still see no reason to raise an exception in a situation that causes 
no problem. Your use of the word "typically" indicates that other scenarios 
*may* be valid, and the current implementation needlessly prevents them.

More to the point (or possibly less so), my situation is not of library code 
installing the reactor. It the case of a GUI service that may be run as part 
of a larger application, but I also want it to be runnable by simply executing 
the main module, using the """if __name__ == "__main__":""" idiom. This 
requires the wxreactor to be installed, but the reactor is imported at the top 
of the script, so I would rather install it there, but in case I use a control 
script to bundle it up in a larger application, this would raise the 
AssertionError. I think it's a pretty legitimate scenario, and even if it's 
not ideal coding, I don't see why it should be disallowed altogether. Anyway, 
what I said in the previous paragraph probably applies to other situations as 
well.

comment:4 Changed 16 years ago by itamarst

Possibly. 

Your patch is probably insufficient though - for one thing, it doesn't check
that it's the same reactor (what happens if someone installs both gtk2 and wx
reactors? One of them will be silently ignored, masking the bug for a while.)
Also, creating a reactor instance can have side effects inside the low-level GUI
event loop which means you can't create the object twice. So the correct thing
to do is check that installed reactor is the same as one you are attempting to
install, if it is the same do not create 2nd reactor instance at all, if it is
different complain.

comment:5 Changed 16 years ago by antony

I'm pretty sure my patch does this.

comment:6 Changed 16 years ago by itamarst

It doesn't - notice that by the time you his this function the second reactor
instance has already been created.

comment:7 Changed 16 years ago by antony

Yes, you're right. I give up then.

comment:8 Changed 16 years ago by TimothyFitz

Resolution: invalid
Status: newclosed

comment:9 Changed 11 years ago by <automation>

Note: See TracTickets for help on using tickets.