Bug 35828

Summary: python: Remove non-trivial code from webkitpy/__init__.py
Product: WebKit Reporter: Chris Jerdonek <cjerdonek>
Component: Tools / TestsAssignee: Chris Jerdonek <cjerdonek>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, eric, hamaji, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 31533    
Attachments:
Description Flags
Proposed patch abarth: review+, cjerdonek: commit-queue-

Description Chris Jerdonek 2010-03-06 05:03:48 PST
The __init__.py file of the top-level webkitpy package currently causes non-trivial code to execute (namely autoinstall.py code).

This report is to remove non-trivial code like this from webkitpy/__init__.py.  This allows calling code to import boot-strapping code like version-checking code and logging configuration code from webkitpy before any errors or log messages caused by code in __init__.py.

In particular, callers will be able to do things like check and warn users about their Python version before erroring out on autoinstall, and configure logging prior to autoinstall so that autoinstall log messages can render.

Another benefit is that autoinstall code can be placed in a location where it only executes if it is actually needed, instead of executing any time someone imports anything from webkitpy.
Comment 1 Chris Jerdonek 2010-03-06 05:38:28 PST
Created attachment 50151 [details]
Proposed patch
Comment 2 Adam Barth 2010-03-13 02:08:22 PST
Comment on attachment 50151 [details]
Proposed patch

Ok.  I'm slightly sad that we don't say "mechanize" anywhere when we import Browser.  Is there some way to say:

from webkitpy.thirdparty.autoinstalled.mechanize import Browser

That seems the most natural to me.

(In any case, this patch is an improvement.)
Comment 3 Chris Jerdonek 2010-03-13 13:43:45 PST
(In reply to comment #2)
> (From update of attachment 50151 [details])
> Ok.  I'm slightly sad that we don't say "mechanize" anywhere when we import
> Browser. 

Yeah, I agree.

> Is there some way to say:
> 
> from webkitpy.thirdparty.autoinstalled.mechanize import Browser

I tried this and couldn't seem to get it to work -- probably because mechanize.py doesn't actually exist in thirdparty/autoinstalled.

The closest I could come was this:

> import webkitpy.thirdparty.autoinstalled.mechanize as mechanize
> Browser = mechanize.Browser

But that takes two lines in each calling file which seems less preferable.

In any case, the good news is that this is a temporary measure.  It will go away once I re-submit the autoinstall rewrite.  I'll be doing that once I land all this version-checking stuff.
Comment 4 Chris Jerdonek 2010-03-13 14:11:42 PST
Manually committed (via git svn dcommit):

http://trac.webkit.org/changeset/55969