Summary: | fix http_server --server start | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||||||||
Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cjerdonek, commit-queue, ojan, tony | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Dirk Pranke
2010-02-23 15:40:30 PST
Created attachment 49332 [details]
Patch
Created attachment 49342 [details]
revised patch - fix merge properly
Created attachment 49343 [details]
remove extraneous import from apache_http_server.py
Ojan, can you take a look at this and make sure it looks right? This seems sane to me. I don't really know how __init__ works though. Tony, does this look right to you? Is it possible to "import port" instead? You may have to muck around with the include path. Importing __init__ directly feels wrong. The other way to do this is to move the get() method from __init__ into a different file (like base.py) and have __init__.py "import get from base". Then you can use base.get from http_server.py. "import port" doesn't work w/o messing w/ the include path (and I don't want to do that because it feels really wrong). "import get from base" might be the way to go. (In reply to comment #6) > Is it possible to "import port" instead? You may have to muck around with the > include path. Importing __init__ directly feels wrong. > > The other way to do this is to move the get() method from __init__ into a > different file (like base.py) and have __init__.py "import get from base". > Then you can use base.get from http_server.py. A few things-- (1) It looks like import __init__ is now listed twice. (2) You shouldn't ever need to import __init__ directly. Tony is right. __init__ is what gets imported when you import the directory containing __init__.py. (3) If you want to import a method inside __init__.py, then you can simply "from port import get", for example. I did this recently in the following patch: https://bugs.webkit.org/show_bug.cgi?id=35162 Here I defined console_log_handler in webkitpy's __init__.py, so I was able to type "from webkitpy import console_log_handler" in a different file. (In reply to comment #7) > "import port" doesn't work w/o messing w/ the include path (and I don't want to > do that because it feels really wrong). > > "import get from base" might be the way to go. You probably shouldn't need to modify sys.path. A couple possibilities are to use relative imports: from ..port import get (I think this is the syntax for you in this case. See here, for example-- http://docs.python.org/tutorial/modules.html#intra-package-references ) Or more preferable is to use absolute imports (according to PEP8). Since webkitpy is already in your search I path I assume, you should just be able to do-- from webkitpy.layout_tests.port import get (In reply to comment #8) > A few things-- > > (1) It looks like import __init__ is now listed twice. Sorry, I was looking at your first patch here, not your latest one. You might also want to try-- from . import get This might work to import get from your current directory's __init__.py, without having to use a relative import one level up (i.e. "from ..port"). (In reply to comment #10) > (In reply to comment #8) > > A few things-- > > > > (1) It looks like import __init__ is now listed twice. > > Sorry, I was looking at your first patch here, not your latest one. > > You might also want to try-- > > from . import get > > This might work to import get from your current directory's __init__.py, > without having to use a relative import one level up (i.e. "from ..port"). Unfortunately, relative imports don't work in python 2.4, which is what Chromium uses on Windows. Moreover, I didn't particularly like the idea of importing the "get" symbol as an unqualified name. So, I moved the "get" into a new file (factory.py). I didn't move it into base.py to avoid the appearance of circular dependencies, and just to avoid confusing things. Revised patch forthcoming. Created attachment 49422 [details]
move __init__.get() into a new file (factory.py)
Comment on attachment 49422 [details]
move __init__.get() into a new file (factory.py)
Consider: Would be nice to have a little more in the changelog on a per file basis to explain why the changes were done in that file as discussed.
Fix landed as r55210. Closing bug. |