Skip to content

Commit 32444b5

Browse files
committed
Exit with a status code if Fire encounters an error
Copybara generated commit for Python Fire. PiperOrigin-RevId: 150818132 Change-Id: Ice3b2977fe0e89fa6472f0831e7475f3318943cf Reviewed-on: https://team-review.git.corp.google.com/65029 Reviewed-by: David Bieber <dbieber@google.com>
1 parent babf4f1 commit 32444b5

File tree

4 files changed

+73
-37
lines changed

4 files changed

+73
-37
lines changed

fire/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
"""Python Fire module for third_party."""
15+
"""The Python Fire module."""
1616

1717
from __future__ import absolute_import
1818
from __future__ import division
1919
from __future__ import print_function
2020

2121
from fire.core import Fire
22+
from fire.core import FireExit
2223

23-
__all__ = ['Fire']
24+
__all__ = ['Fire', 'FireExit']

fire/core.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ def Fire(component=None, command=None, name=None):
9292
it's a class). When all arguments are consumed and there's no function left
9393
to call or class left to instantiate, the resulting current component is
9494
the final result.
95-
If a Fire error is encountered, the Fire Trace is displayed to stdout and
96-
None is returned.
9795
If the trace command line argument is supplied, the FireTrace is returned.
96+
Raises:
97+
FireExit: If a FireError is countered.
9898
"""
9999
# Get args as a list.
100100
if command is None:
@@ -127,7 +127,7 @@ def Fire(component=None, command=None, name=None):
127127
result = component_trace.GetResult()
128128
print(
129129
helputils.HelpString(result, component_trace, component_trace.verbose))
130-
return None
130+
raise FireExit(1, component_trace)
131131
elif component_trace.show_trace and component_trace.show_help:
132132
print('Fire trace:\n{trace}\n'.format(trace=component_trace))
133133
result = component_trace.GetResult()
@@ -161,6 +161,20 @@ class FireError(Exception):
161161
"""
162162

163163

164+
class FireExit(SystemExit):
165+
"""An exception raised by Fire to the client in the case of a FireError.
166+
167+
Contains the trace of the Fire program.
168+
169+
If not caught, then a FireExit will cause the program to exit with a non-zero
170+
status code.
171+
"""
172+
173+
def __init__(self, status, data):
174+
super(FireExit, self).__init__(status)
175+
self.trace = data
176+
177+
164178
def _PrintResult(component_trace, verbose=False):
165179
"""Prints the result of the Fire call to stdout in a human readable way."""
166180
# TODO: Design human readable deserializable serialization method
@@ -558,8 +572,8 @@ def _ParseFn(args):
558572

559573
# Note: _ParseArgs modifies kwargs.
560574
parsed_args, kwargs, remaining_args, capacity = _ParseArgs(
561-
fn_spec.args, fn_spec.defaults, num_required_args, kwargs, remaining_args,
562-
metadata)
575+
fn_spec.args, fn_spec.defaults, num_required_args, kwargs,
576+
remaining_args, metadata)
563577

564578
if fn_spec.varargs or fn_spec.varkw:
565579
# If we're allowed *varargs or **kwargs, there's always capacity.

fire/core_test.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ def testInteractiveModeVariablesWithName(self, mock_embed):
6767

6868
def testImproperUseOfHelp(self):
6969
# This should produce a warning and return None.
70-
self.assertIsNone(core.Fire(tc.TypedProperties, 'alpha --help'))
70+
with self.assertRaises(core.FireExit):
71+
core.Fire(tc.TypedProperties, 'alpha --help')
7172

7273
def testErrorRaising(self):
7374
# Errors in user code should not be caught; they should surface as normal.

fire/fire_test.py

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,13 @@ def testFireNoArgs(self):
4141
self.assertEqual(fire.Fire(tc.MixedDefaults, 'ten'), 10)
4242

4343
def testFireExceptions(self):
44-
# Exceptions of Fire are printed to stderr and None is returned.
45-
self.assertIsNone(fire.Fire(tc.Empty, 'nomethod')) # Member doesn't exist.
46-
self.assertIsNone(fire.Fire(tc.NoDefaults, 'double')) # Missing argument.
47-
self.assertIsNone(fire.Fire(tc.TypedProperties, 'delta x')) # Missing key.
44+
# Exceptions of Fire are printed to stderr and a FireExit is raised.
45+
with self.assertRaises(fire.FireExit):
46+
fire.Fire(tc.Empty, 'nomethod') # Member doesn't exist.
47+
with self.assertRaises(fire.FireExit):
48+
fire.Fire(tc.NoDefaults, 'double') # Missing argument.
49+
with self.assertRaises(fire.FireExit):
50+
fire.Fire(tc.TypedProperties, 'delta x') # Missing key.
4851

4952
# Exceptions of the target components are still raised.
5053
with self.assertRaises(ZeroDivisionError):
@@ -89,11 +92,13 @@ def testFirePartialNamedArgs(self):
8992
fire.Fire(tc.MixedDefaults, 'identity --beta 1 --alpha 2'), (2, 1))
9093

9194
def testFirePartialNamedArgsOneMissing(self):
92-
# By default, errors are written to standard out and None is returned.
93-
self.assertIsNone( # Identity needs an arg.
94-
fire.Fire(tc.MixedDefaults, 'identity'))
95-
self.assertIsNone( # Identity needs a value for alpha.
96-
fire.Fire(tc.MixedDefaults, 'identity --beta 2'))
95+
# Errors are written to standard out and a FireExit is raised.
96+
with self.assertRaises(fire.FireExit):
97+
fire.Fire(tc.MixedDefaults, 'identity') # Identity needs an arg.
98+
99+
with self.assertRaises(fire.FireExit):
100+
# Identity needs a value for alpha.
101+
fire.Fire(tc.MixedDefaults, 'identity --beta 2')
97102

98103
self.assertEqual(fire.Fire(tc.MixedDefaults, 'identity 1'), (1, '0'))
99104
self.assertEqual(
@@ -103,9 +108,11 @@ def testFireAnnotatedArgs(self):
103108
self.assertEqual(fire.Fire(tc.Annotations, 'double 5'), 10)
104109
self.assertEqual(fire.Fire(tc.Annotations, 'triple 5'), 15)
105110

106-
@unittest.skipIf(six.PY2, 'Keyword-only arguments not supported in Python 2')
111+
@unittest.skipIf(six.PY2, 'Keyword-only arguments not in Python 2.')
107112
def testFireKeywordOnlyArgs(self):
108-
self.assertIsNone(fire.Fire(tc.py3.KeywordOnly, 'double 5'))
113+
with self.assertRaises(fire.FireExit):
114+
# Keyword arguments must be passed with flag syntax.
115+
fire.Fire(tc.py3.KeywordOnly, 'double 5')
109116

110117
self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'double --count 5'), 10)
111118
self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'triple --count 5'), 15)
@@ -252,17 +259,19 @@ def fn1(thing, nothing):
252259
self.assertEqual(fire.Fire(fn1, '--thing --nothing'), (True, True))
253260
self.assertEqual(fire.Fire(fn1, '--thing --nonothing'), (True, False))
254261

255-
# In the next example nothing=False (since rightmost setting of a flag gets
256-
# precedence), but it errors because thing has no value.
257-
self.assertEqual(fire.Fire(fn1, '--nothing --nonothing'), None)
262+
with self.assertRaises(fire.FireExit):
263+
# In this case nothing=False (since rightmost setting of a flag gets
264+
# precedence), but it errors because thing has no value.
265+
fire.Fire(fn1, '--nothing --nonothing')
258266

259267
# In these examples, --nothing sets thing=False:
260268
def fn2(thing, **kwargs):
261269
return thing, kwargs
262270
self.assertEqual(fire.Fire(fn2, '--thing'), (True, {}))
263271
self.assertEqual(fire.Fire(fn2, '--nothing'), (False, {}))
264-
# In the next one, nothing=True, but it errors because thing has no value.
265-
self.assertEqual(fire.Fire(fn2, '--nothing=True'), None)
272+
with self.assertRaises(fire.FireExit):
273+
# In this case, nothing=True, but it errors because thing has no value.
274+
fire.Fire(fn2, '--nothing=True')
266275
self.assertEqual(fire.Fire(fn2, '--nothing --nothing=True'),
267276
(False, {'nothing': True}))
268277

@@ -323,7 +332,8 @@ def testBasicSeparator(self):
323332
('-', '_'))
324333

325334
# The separator triggers a function call, but there aren't enough arguments.
326-
self.assertEqual(fire.Fire(tc.MixedDefaults, 'identity - _ +'), None)
335+
with self.assertRaises(fire.FireExit):
336+
fire.Fire(tc.MixedDefaults, 'identity - _ +')
327337

328338
def testNonComparable(self):
329339
"""Fire should work with classes that disallow comparisons."""
@@ -367,24 +377,34 @@ def testFloatForExpectedInt(self):
367377
def testClassInstantiation(self):
368378
self.assertIsInstance(fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2'),
369379
tc.InstanceVars)
370-
# Cannot instantiate a class with positional args by default.
371-
self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2'))
380+
with self.assertRaises(fire.FireExit):
381+
# Cannot instantiate a class with positional args.
382+
fire.Fire(tc.InstanceVars, 'a1 a2')
372383

373384
def testTraceErrors(self):
374385
# Class needs additional value but runs out of args.
375-
self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1'))
376-
self.assertIsNone(fire.Fire(tc.InstanceVars, '--arg1=a1'))
386+
with self.assertRaises(fire.FireExit):
387+
fire.Fire(tc.InstanceVars, 'a1')
388+
with self.assertRaises(fire.FireExit):
389+
fire.Fire(tc.InstanceVars, '--arg1=a1')
390+
377391
# Routine needs additional value but runs out of args.
378-
self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - run b1'))
379-
self.assertIsNone(
380-
fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1'))
392+
with self.assertRaises(fire.FireExit):
393+
fire.Fire(tc.InstanceVars, 'a1 a2 - run b1')
394+
with self.assertRaises(fire.FireExit):
395+
fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1')
396+
381397
# Extra args cannot be consumed.
382-
self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - run b1 b2 b3'))
383-
self.assertIsNone(
384-
fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1 b2 b3'))
398+
with self.assertRaises(fire.FireExit):
399+
fire.Fire(tc.InstanceVars, 'a1 a2 - run b1 b2 b3')
400+
with self.assertRaises(fire.FireExit):
401+
fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1 b2 b3')
402+
385403
# Cannot find member to access.
386-
self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - jog'))
387-
self.assertIsNone(fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - jog'))
404+
with self.assertRaises(fire.FireExit):
405+
fire.Fire(tc.InstanceVars, 'a1 a2 - jog')
406+
with self.assertRaises(fire.FireExit):
407+
fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - jog')
388408

389409

390410
if __name__ == '__main__':

0 commit comments

Comments
 (0)