Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trial and twistd accept inconsistent values for --reactor #11377

Closed
twisted-trac opened this issue Jul 14, 2003 · 21 comments
Closed

trial and twistd accept inconsistent values for --reactor #11377

twisted-trac opened this issue Jul 14, 2003 · 21 comments

Comments

@twisted-trac
Copy link

ne1uno's avatar ne1uno reported
Trac ID trac#69
Type enhancement
Created 2003-07-14 12:34:46Z

Attachments:

  • reactor.diff (4365 bytes) - added by davep on 2003-07-24 07:18:06Z -
  • reactor.2.diff (7498 bytes) - added by davep on 2003-07-28 12:50:37Z -
  • reactor.3.diff (2673 bytes) - added by davep on 2003-08-04 05:18:19Z -
Searchable metadata
trac-id__69 69
type__enhancement enhancement
reporter__ne1uno ne1uno
priority__normal normal
milestone__ 
branch__ 
branch_author__ 
status__closed closed
resolution__fixed fixed
component__ 
keywords__ 
time__1058186086000000 1058186086000000
changetime__1064926641000000 1064926641000000
version__ 
owner__ 
cc__davep cc__itamarst cc__ne1uno cc__warner cc__moshez
@twisted-trac
Copy link
Author

ne1uno's avatar ne1uno commented
#!html
<pre>
$ python trial.py --package=twisted.test --reactor=win32 --
verbose 
$ python trial.py --reactor=win32 --verbose  --
package=twisted.test
Using twisted.internet.win32 reactor
Traceback (most recent call last):
  File "trial.py", line 35, in ?
    run()
  File "L:\c\Python22\Lib\site-
packages\twisted\scripts\trial.py", line 213, in
run
    config.parseOptions()
  File "L:\c\Python22\Lib\site-
packages\twisted\python\usage.py", line 192, in p
arseOptions
    self.__dispatch[optMangled](optMangled, arg)
  File "L:\c\Python22\Lib\site-
packages\twisted\python\usage.py", line 343, in &lt;
lambda>
    fn = lambda self, name, value, m=method: m(value)
  File "L:\c\Python22\Lib\site-
packages\twisted\scripts\trial.py", line 63, in o
pt_reactor
    reflect.namedModule(mod).install()
  File "L:\c\Python22\Lib\site-
packages\twisted\python\reflect.py", line 369, in
 namedModule
    topLevel = __import__(name)
ImportError: No module named win32

> l:\c\python22\lib\site-packages\twisted\python\reflect.py
(369)namedModule()
-> topLevel = __import__(name)
(Pdb)

hope its not user error on my part.

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
You need to pass "--reactor==win32eventreactor". The issue
is that the command line argument is different than
twistd's. Not sure why!

</pre>

@twisted-trac
Copy link
Author

radix's avatar @radix commented
#!html
<pre>
It's that way because maintaining dictionaries mapping names
to reactor modules is annoying (I implemented this
functionality of trial :).

It would be nice to move twistd's mapping to somewhere in
twisted.internet, and then have twistd and trial both use
it. This is a non-bug, though, so I'm marking it as wishlist.

</pre>

@twisted-trac
Copy link
Author

ne1uno's avatar ne1uno commented
#!html
<pre>
obviously the --help needs updating then.
thanks.
btw, color output is fine on win9x if you load the
driver in config.sys
device= C:\Windows\COMMAND\ANSI.SYS

there are other problems running trial from msys the bash 
shell for mingw but thats for another issue.

</pre>

@twisted-trac
Copy link
Author

radix's avatar @radix commented
#!html
<pre>
dunno why this was set to "bug" and "resolved", I put it
back to "feature" and "chatting"...

</pre>

@twisted-trac
Copy link
Author

radix's avatar @radix commented
#!html
<pre>
dunno why this was set to "bug" and "resolved", or why the
title reverted... I'm unreverting those changes

</pre>

@twisted-trac
Copy link
Author

jdavisp3's avatar @jdavisp3 commented
#!html
<pre>
Just fooling around, here's a patch to fix this.

There didn't seem to be an obvious module to put
the table in, so I made a new one twisted.internet.reactors.

I made trial support both the old and new style reactor
arguments for backwards compatibility.

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
A better way to do backwards compatability is to just add
more entries to the dictionaty, rather than doing the
loadModule code. Other than that, the patch looks pretty
good, having a central registry is good (We can add
registration function later on, so 3rd parties can register
their own reactors.)

</pre>

@twisted-trac
Copy link
Author

jdavisp3's avatar @jdavisp3 commented
#!html
<pre>
One problem with adding the old names to the dictionary
is that the dictionary is used to print out a list of
reactor names for help. It would be confusing to print
out two names for each reactor, I think. I could extend
the dictionary to distinguish between canonical names and
synonyms, perhaps.

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
A synonyms dict would work, or having main one map from
{canonical-name: (module, [synonym1, synonym2...]}.

</pre>

@twisted-trac
Copy link
Author

jdavisp3's avatar @jdavisp3 commented
#!html
<pre>
Here's a new patch with a synonym table. I also added
some API for getting the reactor names and installing
reactors by name.

I notice Moshe's latest twistd refactor is unix-only.
Perhaps the reactor table needs to distinguish reactors
based on platform?

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
Moshe, please comment.

</pre>

@twisted-trac
Copy link
Author

m@...'s avatar m@... commented
#!html
<pre>
On Mon, 28 Jul 2003, Itamar Shtull-Trauring &lt;twisted.roundup@...> wrote:

> Moshe, please comment.

If we decide to fix it at all, which I'm not sure is important as
long as reactors are documented in both manpages, we should made
trial to fit twistd, possibly via getting the dictionary in scripts.twistd 
-- 
Moshe Zadka -- http://moshez.org/
Buffy: I don't like you hanging out with someone that... short.
Riley: Yeah, a lot of young people nowadays are experimenting with shortness.
Agile Programming Language -- http://www.python.org/
</pre>

@twisted-trac
Copy link
Author

jdavisp3's avatar @jdavisp3 commented
#!html
<pre>
Here's a much simpler patch that uses the dictionary
from scripts.twistd in trial, also supporting b/c.
The patch to usage.py is still necessary to fix the
help output of trial.

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
We'll revisit this once moshez merges his app refactor
post-1.0.7.

</pre>

@twisted-trac
Copy link
Author

jdavisp3's avatar @jdavisp3 commented
#!html
<pre>
Sounds good. I'm going to commit the usage.py patch
since it fixes the help display of the current version
of trial as well.

</pre>

@twisted-trac
Copy link
Author

itamarst's avatar @itamarst commented
#!html
<pre>
New application package is in, so we should finish dealing
with this issue.

</pre>

@twisted-trac
Copy link
Author

moshez's avatar @moshez commented
#!html
<pre>
Basically, this just requires sticking a call to 
twisted.application.app.installReactor in 
twisted.scripts.trial -- modulo thinking of backwards 
compat. Since Brian is the heaviest automated user, he is 
taking responsibility for this.

</pre>

@twisted-trac
Copy link
Author

warner's avatar @warner commented
#!html
<pre>
I've changed trial to use app.installReactor, and
implemented the documentation update from davep's patch. The
BuildBot has been updated to use the new reactor names when
it runs the "reactors" series of tests.

</pre>

@twisted-trac
Copy link
Author

warner's avatar @warner commented
#!html
<pre>
buildbot is running all the correct tests

</pre>

@twisted-trac
Copy link
Author

Automation's avatar Automation removed owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant