There are a few parts of resultsdbpy that should really be broken into independent packages because other services need them. The first step in that process is for resultsdbpy to depend on the library that some tooling is to be moved to.
<rdar://problem/67250272>
Created attachment 406720 [details] Patch
Created attachment 406721 [details] Patch
Comment on attachment 406721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406721&action=review > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/__init__.py:25 > + Better to make below code a method, like: https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/steps_unittest.py#L17 _add_webkitpy_to_sys_path() > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/__init__.py:26 > +# Hopefully we're beside webkitcorepy, otherwise it will need to be installed 'it' is ambiguous. Please replace with 'webkitcorepy'. Nit: missing full-stop at the end. Also, can you check if it's install and print a nice message if it isn't installed? > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/__init__.py:27 > +containing = os.path.dirname(os.path.dirname(os.path.abspath(os.path.dirname(__file__)))) 'containing' should be renamed to name of actual folder, e.g.: libraries_path > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/__init__.py:28 > +path_addition = os.path.join(containing, 'webkitcorepy') should be 'webkitcorepy_path' 'addition' seems out of context.
Comment on attachment 406721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406721&action=review >> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/__init__.py:26 >> +# Hopefully we're beside webkitcorepy, otherwise it will need to be installed > > 'it' is ambiguous. Please replace with 'webkitcorepy'. > Nit: missing full-stop at the end. > > Also, can you check if it's install and print a nice message if it isn't installed? Other than the error message you'll get on line 32 when you try and use the library? When we have this situation with other packages we own, I intend to have sort of "include or auto-install" function, but we obviously can't auto-install the library that has the auto-installer in it....
(In reply to Jonathan Bedard from comment #5) > > Also, can you check if it's install and print a nice message if it isn't installed? > > Other than the error message you'll get on line 32 when you try and use the library? > > When we have this situation with other packages we own, I intend to have sort of "include or auto-install" function, but we obviously can't auto-install the library that has the auto-installer in it.... Yeah, i am not referring to auto-installing webkitcorepy. I am only referring to better error message when import fails. e.g.: https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/test/main.py#L260 Something like: try: import package_name except ImportError: _log.error('Failed to import webkitcorepy:, Please do XYZ to install this package') Although it's ok without custom error message as well.
Created attachment 406729 [details] Patch
Created attachment 406740 [details] Patch for landing
Committed r265771: <https://trac.webkit.org/changeset/265771> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406740 [details].