WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
147082
Install twisted package for http server when necessary.
https://bugs.webkit.org/show_bug.cgi?id=147082
Summary
Install twisted package for http server when necessary.
dewei_zhu
Reported
2015-07-18 22:46:27 PDT
Install twisted package for http server when necessary.
Attachments
Patch
(1.88 KB, patch)
2015-07-18 23:13 PDT
,
dewei_zhu
no flags
Details
Formatted Diff
Diff
Patch
(3.89 KB, patch)
2015-08-04 16:26 PDT
,
dewei_zhu
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
dewei_zhu
Comment 1
2015-07-18 23:13:08 PDT
Created
attachment 257054
[details]
Patch
Ryosuke Niwa
Comment 2
2015-07-19 00:13:44 PDT
Comment on
attachment 257054
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=257054&action=review
> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12 > + from webkitpy.common.system.autoinstall import AutoInstaller
Why don't we modify __init__.py in autoinstalled instead? What happens if we've already installed twisted in that directory by using buildbot?
dewei_zhu
Comment 3
2015-07-19 14:31:51 PDT
Comment on
attachment 257054
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=257054&action=review
>> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12 >> + from webkitpy.common.system.autoinstall import AutoInstaller > > Why don't we modify __init__.py in autoinstalled instead? > What happens if we've already installed twisted in that directory by using buildbot?
If we've installed twisted, we'll not catch ImportError. If we modify __init__.py, how do we invoke the installation process? Do you mean we import autoinstall module in benchmark runner and it will check the installation of twisted and install it if necessary?
Ryosuke Niwa
Comment 4
2015-07-19 14:46:15 PDT
Comment on
attachment 257054
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=257054&action=review
>>> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:12 >>> + from webkitpy.common.system.autoinstall import AutoInstaller >> >> Why don't we modify __init__.py in autoinstalled instead? >> What happens if we've already installed twisted in that directory by using buildbot? > > If we've installed twisted, we'll not catch ImportError. > If we modify __init__.py, how do we invoke the installation process? > Do you mean we import autoinstall module in benchmark runner and it will check the installation of twisted and install it if necessary?
But that's what we tried other day (importing autoinstall.buildbot) on my machine and we saw import didn't work.
> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py:16 > + installer = AutoInstaller(prepend_to_search_path=True, target_dir=install_path) > + installer.install(url="
https://pypi.python.org/packages/source/T/Twisted/Twisted-12.1.0.tar.bz2#md5=f396f1d6f5321e869c2f89b2196a9eb5
", url_subpath="Twisted-12.1.0/twisted")
There is already code that does this in __init__.py in webkitpy/thirdparty for when we import thirdparty.autoinstall.twisted. Why don't we make that code more generic to do this instead? It appears to me that having to download & install twisted every time you run benchmark is somewhat inefficient/redundant. I already hate the fact some of the benchmark plans require downloading the benchmark and we should add some code to cache the downloaded content.
dewei_zhu
Comment 5
2015-08-04 16:26:15 PDT
Created
attachment 258229
[details]
Patch
WebKit Commit Bot
Comment 6
2015-08-04 17:46:56 PDT
Comment on
attachment 258229
[details]
Patch Clearing flags on attachment: 258229 Committed
r187925
: <
http://trac.webkit.org/changeset/187925
>
WebKit Commit Bot
Comment 7
2015-08-04 17:46:59 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug