Differential D659

Fix module import error masking missing modules
ClosedPublic
Actions

Authored by mkrizek on Nov 20 2015, 6:04 PM.

Details

Reviewers
lbrabec
kparal
tflink
jskladan
Maniphest Tasks
T652: module import error message masks missing modules
Commits
rLTRNf3df144cf681: Fix module import error masking missing modules
Summary

This ensures no missing module error message is lost, encapsulates errors into TaskotronImportError so it doesn't look like a bug (just ImportError) and contains a somewhat user-friendly hint if missing import is from libtaskotron.

Test Plan

Tests amended. Tried importing nonexisting module (both from and outside libtaskotron) from rpm_utils, worked.

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mkrizek retitled this revision from to Fix module import error masking missing modules.Nov 20 2015, 6:04 PM
mkrizek updated this object.
mkrizek edited the test plan for this revision. (Show Details)
mkrizek added reviewers: tflink, kparal, jskladan, lbrabec.
jskladan accepted this revision.Nov 23 2015, 9:03 AM
This revision is now accepted and ready to land.Nov 23 2015, 9:03 AM
tflink accepted this revision.Nov 24 2015, 6:30 PM
Closed by commit rLTRNf3df144cf681: Fix module import error masking missing modules (authored by mkrizek). · Explain WhyNov 25 2015, 9:14 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1717

View Options

libtaskotron/exceptions.py

Show All 37 Lines
38 38​
39 39​
40class TaskotronNotImplementedError(TaskotronError, NotImplementedError): 40​class TaskotronNotImplementedError(TaskotronError, NotImplementedError):
41 '''NotImplementedError for Taskotron classes, methods and functions''' 41​ '''NotImplementedError for Taskotron classes, methods and functions'''
42 42​
43 43​
44class TaskotronImportError(TaskotronError): 44​class TaskotronImportError(TaskotronError):
45 '''All issues with Extensions''' 45​ '''All issues with Extensions'''
46 pass 46​
47​ def __init__(self, e):
48​ msg = str(e.message)
49​ if 'libtaskotron' in msg:
50​ msg += "\nHint: Make sure that all required sub-packages are installed."
51​ super(TaskotronError, self).__init__(msg)
47 52​
48 53​
49class TaskotronImageError(TaskotronError): 54​class TaskotronImageError(TaskotronError):
50 '''All generic image related errors''' 55​ '''All generic image related errors'''
51 pass 56​ pass
52 57​
53 58​
54class TaskotronImageNotFoundError(TaskotronImageError): 59​class TaskotronImageNotFoundError(TaskotronImageError):
55 '''Requested image not found error''' 60​ '''Requested image not found error'''
56 pass 61​ pass
View Options

libtaskotron/executor.py

Show All 15 Lines
16from libtaskotron import python_utils 16​from libtaskotron import python_utils
17from libtaskotron import config 17​from libtaskotron import config
18from libtaskotron.logger import log 18​from libtaskotron.logger import log
19import libtaskotron.exceptions as exc 19​import libtaskotron.exceptions as exc
20from libtaskotron.directives import exitcode_directive 20​from libtaskotron.directives import exitcode_directive
21 21​
22try: 22​try:
23 from libtaskotron.ext.fedora import rpm_utils 23​ from libtaskotron.ext.fedora import rpm_utils
24except ImportError: 24​except ImportError, e:
25 raise exc.TaskotronImportError("Executor requires libtaskotron.ext.fedora.rpm_utils but " 25​ raise exc.TaskotronImportError(e)
26 "it could not be imported. Make sure that " 26​
27 "libtaskotron-fedora is installed") ​
28 27​
29class Executor(object): 28​class Executor(object):
30 '''This class serves the purpose of actual task execution. It is instantiated by 29​ '''This class serves the purpose of actual task execution. It is instantiated by
31 :class:`.Overlord`, :class:`.PersistentMinion` or :class:`.DisposableMinion`. 30​ :class:`.Overlord`, :class:`.PersistentMinion` or :class:`.DisposableMinion`.
32 ''' 31​ '''
33 32​
34 def __init__(self, task_data, arg_data, workdir=None): 33​ def __init__(self, task_data, arg_data, workdir=None):
35 '''Create a new instance. 34​ '''Create a new instance.
▲ Show 20 Lines • Show All 163 Lines • ▼ Show 20 Line(s)190 def _load_directive(self, directive_name, directive_dir=None):
199 198​
200 if not os.path.exists(directive_file): 199​ if not os.path.exists(directive_file):
201 raise exc.TaskotronDirectiveError("Directive %s not found in directory %s" % 200​ raise exc.TaskotronDirectiveError("Directive %s not found in directory %s" %
202 (directive_name, directive_dir)) 201​ (directive_name, directive_dir))
203 202​
204 try: 203​ try:
205 loaded_directive = imp.load_source(real_name, directive_file) 204​ loaded_directive = imp.load_source(real_name, directive_file)
206 except ImportError, e: 205​ except ImportError, e:
207 if e.message.find('libtaskotron') >= 0: 206​ raise exc.TaskotronImportError(e)
208 log.error("Error while loading directive {}: make sure that all required " ​
209 "sub-packages are installed.".format(directive_name)) ​
210 raise exc.TaskotronImportError(e) ​
211 else: ​
212 raise e ​
213 207​
214 self.directives[directive_name] = loaded_directive 208​ self.directives[directive_name] = loaded_directive
215 209​
216 def _validate_env(self): 210​ def _validate_env(self):
217 # TODO: implement this 211​ # TODO: implement this
218 raise NotImplementedError("Environment validation is not yet implemented") 212​ raise NotImplementedError("Environment validation is not yet implemented")
View Options

libtaskotron/minion.py

Show All 10 Lines
11from libtaskotron import logger 11​from libtaskotron import logger
12from libtaskotron import config 12​from libtaskotron import config
13from libtaskotron import remote_exec 13​from libtaskotron import remote_exec
14from libtaskotron.logger import log 14​from libtaskotron.logger import log
15import libtaskotron.exceptions as exc 15​import libtaskotron.exceptions as exc
16 16​
17try: 17​try:
18 from libtaskotron.ext.disposable import vm 18​ from libtaskotron.ext.disposable import vm
19except ImportError: 19​except ImportError, e:
20 raise exc.TaskotronImportError("Minion requires libtaskotron.ext.disposable.vm but " 20​ raise exc.TaskotronImportError(e)
21 "it could not be imported. Make sure that " ​
22 "libtaskotron-disposable is installed") ​
23 ​
24 21​
25 22​
26class BaseMinion(object): 23​class BaseMinion(object):
27 '''Base Minion class that shouldn't be used on its own, it solely initiates inner attributes 24​ '''Base Minion class that shouldn't be used on its own, it solely initiates inner attributes
28 and environment. It also provides method _run() that delegates the task execution to Executor 25​ and environment. It also provides method _run() that delegates the task execution to Executor
29 over SSH. 26​ over SSH.
30 ''' 27​ '''
31 28​
▲ Show 20 Lines • Show All 157 Lines • Show Last 20 Lines
View Options

testing/functest_executor.py

Show All 35 Lines34 def test_executor_load_directive_notexist(self, tmpdir):
36 36​
37 test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir) 37​ test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir)
38 38​
39 with pytest.raises(exc.TaskotronDirectiveError): 39​ with pytest.raises(exc.TaskotronDirectiveError):
40 test_executor._load_directive(self.ref_directivename, self.ref_dir) 40​ test_executor._load_directive(self.ref_directivename, self.ref_dir)
41 41​
42 def test_executor_load_directive_our_importerror(self, tmpdir, monkeypatch): 42​ def test_executor_load_directive_our_importerror(self, tmpdir, monkeypatch):
43 """ The idea here is to test the logic that looks for an ImportError 43​ """ The idea here is to test the logic that looks for an ImportError
44 during directive loading and only transforms the exception if it's coming 44​ during directive loading, transforms the exception and includes a hint
45 from libtaskotron 45​ to install sub-packages if it's coming from libtaskotron
46 """ 46​ """
47 47​
48 stub_imp = Mock(side_effect=(ImportError("No module libtaskotron.stub"))) 48​ stub_imp = Mock(side_effect=(ImportError("No module libtaskotron.stub")))
49 monkeypatch.setattr(imp, 'load_source', stub_imp) 49​ monkeypatch.setattr(imp, 'load_source', stub_imp)
50 50​
51 test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir) 51​ test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir)
52 52​
53 with pytest.raises(exc.TaskotronImportError): 53​ with pytest.raises(exc.TaskotronImportError) as e:
54 test_executor._load_directive(self.ref_directivename, self.ref_dir) 54​ test_executor._load_directive(self.ref_directivename, self.ref_dir)
55​ assert 'Hint' in e.value.message
55 56​
56 def test_executor_load_directive_notour_importerror(self, tmpdir, monkeypatch): 57​ def test_executor_load_directive_notour_importerror(self, tmpdir, monkeypatch):
57 """ The opposite case as out_importerror - make sure that the ImportError 58​ """ The similar case as out_importerror just check that the exception message
58 is not changed into a Taskotron exception if it's not from libtaskotron 59​ does not include a hint
59 """ 60​ """
60 stub_imp = Mock(side_effect=(ImportError("No module unicorn.candy"))) 61​ stub_imp = Mock(side_effect=(ImportError("No module unicorn.candy")))
61 monkeypatch.setattr(imp, 'load_source', stub_imp) 62​ monkeypatch.setattr(imp, 'load_source', stub_imp)
62 63​
63 test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir) 64​ test_executor = executor.Executor(self.ref_taskdata, self.ref_inputdata, tmpdir)
64 65​
65 with pytest.raises(ImportError): 66​ with pytest.raises(exc.TaskotronImportError) as e:
66 test_executor._load_directive(self.ref_directivename, self.ref_dir) 67​ test_executor._load_directive(self.ref_directivename, self.ref_dir)
68​ assert 'Hint' not in e.value.message