Currently client/setup_modules.py has an undefined variable error that was spot by Amos Kong <akong@xxxxxxxxxx>, and a simple fix was proposed. (see http://patchwork.test.kernel.org/patch/1315/) It seems to me that in a previous refactor of setup_modules, some code was moved but not all references were properly fixed. Amos's fix, while fine, introduces global variables to accomplish the goal and didn't fix another undefined reference that was lying around. As in general it's advisable to avoid resorting to global namespacing, I propose another fix: Encapsulate the library namespace setup into a class, and make setup() just a delegate of the setup() method inside that class. This way it's possible to encapsulate code and use class attributes to avoid using global variables. Since I was there I modified the docstrings of that file to comply with the docstring guideline. John, please let me know what you think about this. Complexity: Medium (Code refactor, all public API preserved) Risk: High (Core code executed by the client). Change was tested though :) Signed-off-by: Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx> --- client/setup_modules.py | 306 ++++++++++++++++++++++++++++------------------ 1 files changed, 186 insertions(+), 120 deletions(-) diff --git a/client/setup_modules.py b/client/setup_modules.py index dc255c4..188114c 100644 --- a/client/setup_modules.py +++ b/client/setup_modules.py @@ -1,4 +1,9 @@ -__author__ = "jadmanski@xxxxxxxxxx (John Admanski)" +""" +Utility library used for autotest library namespace configuration. + +@copyright Google 2007-2009 +@author John Admanski <jadmanski@xxxxxxxxxx> +""" import os, sys @@ -12,134 +17,195 @@ check_version.check_python_version() import new, glob, traceback +class namespace_config(object): + """ + Class to perform namespace configuration for autotest. -def _create_module(name): - """Create a single top-level module""" - module = new.module(name) - sys.modules[name] = module - return module - - -def _create_module_and_parents(name): - """Create a module, and all the necessary parents""" - parts = name.split(".") - # first create the top-level module - parent = _create_module(parts[0]) - created_parts = [parts[0]] - parts.pop(0) - # now, create any remaining child modules - while parts: - child_name = parts.pop(0) - module = new.module(child_name) - setattr(parent, child_name, module) - created_parts.append(child_name) - sys.modules[".".join(created_parts)] = module - parent = module - - -def _import_children_into_module(parent_module_name, path): - """Import all the packages on a path into a parent module""" - # find all the packages at 'path' - names = [] - for filename in os.listdir(path): - full_name = os.path.join(path, filename) - if not os.path.isdir(full_name): - continue # skip files - if "." in filename: - continue # if "." is in the name it's not a valid package name - if not os.access(full_name, os.R_OK | os.X_OK): - continue # need read + exec access to make a dir importable - if "__init__.py" in os.listdir(full_name): - names.append(filename) - # import all the packages and insert them into 'parent_module' - sys.path.insert(0, path) - for name in names: - module = __import__(name) - # add the package to the parent - parent_module = sys.modules[parent_module_name] - setattr(parent_module, name, module) - full_name = parent_module_name + "." + name - sys.modules[full_name] = module - # restore the system path - sys.path.pop(0) + This is what makes possible to use statements like + from autotest_lib.client.bin import package + """ + def __init__(self, base_path, root_module_name=""): + """ + Perform all the necessary setup so that all the packages at + 'base_path' can be imported via "import root_module_name.package". + If root_module_name is empty, then all the packages at base_path + are inserted as top-level packages. + + Also, setup all the common.* aliases for modules in the common + library. + + The setup must be different if you are running on an Autotest server + or on a test machine that just has the client directories installed. + + @param base_path: Path used to compute all relative paths needed for + the setup. + @param root_module_name: Top level name of the 'virtual' library + namespace. + """ + # Hack... Any better ideas? + if (root_module_name == 'autotest_lib.client' and + os.path.exists(os.path.join(os.path.dirname(__file__), + '..', 'server'))): + root_module_name = 'autotest_lib' + base_path = os.path.abspath(os.path.join(base_path, '..')) + self.root_module_name = root_module_name + self.base_path = base_path + + + def _create_module(self, name): + """ + Create a single top-level module. + + @param name: Name of the module you wish to create. + """ + module = new.module(name) + sys.modules[name] = module + return module + + + def _create_module_and_parents(self, name): + """ + Create a module, and all the necessary parents. + + @param name: Name of the module you wish to create. + """ + parts = name.split(".") + # first create the top-level module + parent = self._create_module(parts[0]) + created_parts = [parts[0]] + parts.pop(0) + # now, create any remaining child modules + while parts: + child_name = parts.pop(0) + module = new.module(child_name) + setattr(parent, child_name, module) + created_parts.append(child_name) + sys.modules[".".join(created_parts)] = module + parent = module + + + def _import_children_into_module(self, parent_module_name, path): + """ + Import all the packages on a path into a parent module. + + @param parent_module_name: Name of the parent module you want to use + @param path: Path that contains a number of python packages (libs) + """ + # find all the packages at 'path' + names = [] + for filename in os.listdir(path): + full_name = os.path.join(path, filename) + if not os.path.isdir(full_name): + continue # skip files + if "." in filename: + continue # if "." is in the name it's not a valid package name + if not os.access(full_name, os.R_OK | os.X_OK): + continue # need read + exec access to make a dir importable + if "__init__.py" in os.listdir(full_name): + names.append(filename) + # import all the packages and insert them into 'parent_module' + sys.path.insert(0, path) + for name in names: + module = __import__(name) + # add the package to the parent + parent_module = sys.modules[parent_module_name] + setattr(parent_module, name, module) + full_name = parent_module_name + "." + name + sys.modules[full_name] = module + # restore the system path + sys.path.pop(0) + + + def _autotest_logging_handle_error(self, record): + """ + Method to monkey patch into logging.Handler to replace handleError. + + @param record: Logging record that is going to be treated. + """ + # The same as the default logging.Handler.handleError but also prints + # out the original record causing the error so there is -some- idea + # about which call caused the logging error. + import logging + if logging.raiseExceptions: + # Avoid recursion as the below output can end up back in here when + # something has *seriously* gone wrong in autotest. + logging.raiseExceptions = 0 + sys.stderr.write('Exception occurred formatting message: ' + '%r using args %r\n' % (record.msg, record.args)) + traceback.print_stack() + sys.stderr.write('Future logging formatting exceptions disabled.\n') + + if self.root_module_name == 'autotest_lib': + # Allow locally installed third party packages to be found + # before any that are installed on the system itself when not. + # running as a client. + # This is primarily for the benefit of frontend and tko so that they + # may use libraries other than those available as system packages. + sys.path.insert(0, os.path.join(self.base_path, "site-packages")) + + + def _monkeypatch_logging_handle_error(self): + """ + Hack out logging.py to introduce a custom logging error handling + function. + """ + # Hack out logging.py* + logging_py = os.path.join(os.path.dirname(__file__), "common_lib", + "logging.py*") + if glob.glob(logging_py): + os.system("rm -f %s" % logging_py) + + # Monkey patch our own handleError into the logging module's + # StreamHandler. A nicer way of doing this -might- be to have our own + # logging module define an autotest Logger instance that added our own + # Handler subclass with this handleError method in it. But that would + # mean modifying tons of code. + import logging + assert callable(logging.Handler.handleError) + logging.Handler.handleError = self._autotest_logging_handle_error + + + def setup(self): + """ + Effectively creates the autotest_lib namespace. + """ + # Some internal functions might need to reference those variables. + # It doesn't occur to me how to refactor the API in this file in a clean + # way to avoid + self._create_module_and_parents(self.root_module_name) + self._import_children_into_module(self.root_module_name, self.base_path) + + if self.root_module_name == 'autotest_lib': + # Allow locally installed third party packages to be found + # before any that are installed on the system itself when not. + # running as a client. + # This is primarily for the benefit of frontend and tko so that they + # may use libraries other than those available as system packages. + sys.path.insert(0, os.path.join(self.base_path, "site-packages")) + + self._monkeypatch_logging_handle_error() def import_module(module, from_where): - """Equivalent to 'from from_where import module' - Returns the corresponding module""" + """ + Equivalent to 'from from_where import module' + + @param module: Module intended to be loaded + @param from_where: Top level module where we want to import the module + @return: loaded module + """ from_module = __import__(from_where, globals(), locals(), [module]) return getattr(from_module, module) -def _autotest_logging_handle_error(self, record): - """Method to monkey patch into logging.Handler to replace handleError.""" - # The same as the default logging.Handler.handleError but also prints - # out the original record causing the error so there is -some- idea - # about which call caused the logging error. - import logging - if logging.raiseExceptions: - # Avoid recursion as the below output can end up back in here when - # something has *seriously* gone wrong in autotest. - logging.raiseExceptions = 0 - sys.stderr.write('Exception occurred formatting message: ' - '%r using args %r\n' % (record.msg, record.args)) - traceback.print_stack() - sys.stderr.write('Future logging formatting exceptions disabled.\n') - - if root_module_name == 'autotest_lib': - # Allow locally installed third party packages to be found - # before any that are installed on the system itself when not. - # running as a client. - # This is primarily for the benefit of frontend and tko so that they - # may use libraries other than those available as system packages. - sys.path.insert(0, os.path.join(base_path, "site-packages")) - - -def _monkeypatch_logging_handle_error(): - # Hack out logging.py* - logging_py = os.path.join(os.path.dirname(__file__), "common_lib", - "logging.py*") - if glob.glob(logging_py): - os.system("rm -f %s" % logging_py) - - # Monkey patch our own handleError into the logging module's StreamHandler. - # A nicer way of doing this -might- be to have our own logging module define - # an autotest Logger instance that added our own Handler subclass with this - # handleError method in it. But that would mean modifying tons of code. - import logging - assert callable(logging.Handler.handleError) - logging.Handler.handleError = _autotest_logging_handle_error - - def setup(base_path, root_module_name=""): """ - Perform all the necessary setup so that all the packages at - 'base_path' can be imported via "import root_module_name.package". - If root_module_name is empty, then all the packages at base_path - are inserted as top-level packages. - - Also, setup all the common.* aliases for modules in the common - library. + Configures the autotest_lib namespace for use on a given autotest + entry point - The setup must be different if you are running on an Autotest server - or on a test machine that just has the client directories installed. + @param base_path: Path used to compute all relative paths needed for the + setup. + @param root_module_name: Top level name of the 'virtual' library namespace. """ - # Hack... Any better ideas? - if (root_module_name == 'autotest_lib.client' and - os.path.exists(os.path.join(os.path.dirname(__file__), - '..', 'server'))): - root_module_name = 'autotest_lib' - base_path = os.path.abspath(os.path.join(base_path, '..')) - - _create_module_and_parents(root_module_name) - _import_children_into_module(root_module_name, base_path) - - if root_module_name == 'autotest_lib': - # Allow locally installed third party packages to be found - # before any that are installed on the system itself when not. - # running as a client. - # This is primarily for the benefit of frontend and tko so that they - # may use libraries other than those available as system packages. - sys.path.insert(0, os.path.join(base_path, "site-packages")) - - _monkeypatch_logging_handle_error() + configurator = namespace_config(base_path, root_module_name) + configurator.setup() -- 1.6.2.5 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
