Re: [gentoo-portage-dev] [PATCH] emaint: exit with non-zero status code when module fails (bug 567478)

[Brian Dolbec]

On Tue, 17 Jan 2017 19:48:16 +0200
Alexandru Elisei <alexandru.elisei@×××××.com> wrote:

> Currently module functions return a message to emaint after
> invocation. Emaint prints this message then exits normally (with a
> success return code) even if the module encountered an error. This
> patch aims to change this by having each module public function
> return a tuple of (returncode, message). Emaint will inspect the
> return code and will exit unsuccessfully if necessary.
> ---
>  pym/portage/emaint/main.py                    | 11 +++++++++--
>  pym/portage/emaint/modules/binhost/binhost.py |  6 ++++--
>  pym/portage/emaint/modules/config/config.py   |  8 ++++++--
>  pym/portage/emaint/modules/logs/logs.py       |  9 +++++----
>  pym/portage/emaint/modules/merges/merges.py   | 18 +++++++++++-------
>  pym/portage/emaint/modules/move/move.py       |  9 +++++++--
>  pym/portage/emaint/modules/resume/resume.py   |  4 +++-
>  pym/portage/emaint/modules/sync/sync.py       | 19
> +++++++++++++------ pym/portage/emaint/modules/world/world.py     |
> 8 ++++++-- 9 files changed, 64 insertions(+), 28 deletions(-)
>
> diff --git a/pym/portage/emaint/main.py b/pym/portage/emaint/main.py
> index 65e3545..ef4383a 100644
> --- a/pym/portage/emaint/main.py
> +++ b/pym/portage/emaint/main.py
> @@ -115,6 +115,7 @@ class TaskHandler(object):
>  		"""Runs the module tasks"""
>  		if tasks is None or func is None:
>  			return
> +		returncode = os.EX_OK
>  		for task in tasks:
>  			inst = task()
>  			show_progress = self.show_progress_bar and
> self.isatty @@ -135,14 +136,20 @@ class TaskHandler(object):
>  				# them for other tasks if there is
> more to do. 'options': options.copy()
>  				}
> -			result = getattr(inst, func)(**kwargs)
> +			rcode, msgs = getattr(inst, func)(**kwargs)
>  			if show_progress:
>  				# make sure the final progress is
> displayed self.progress_bar.display()
>  				print()
>  				self.progress_bar.stop()
>  			if self.callback:
> -				self.callback(result)
> +				self.callback(msgs)
> +			# Keep the last error code when using the
> 'all' command.
> +			if rcode != os.EX_OK:
> +				returncode = rcode
> +
> +		if returncode != os.EX_OK:
> +			sys.exit(returncode)
>


Please move this sys.exit to the main().  So this should just return
a list of the rcodes to match the tasks it was passed.
TaskHandler can be used easily via api, so we don't want it to exit out
of someone else's code.  That will also keep main() as the primary cli
interface.  There process the list of rcodes and exit appropriately.



>
>  def print_results(results):
> diff --git a/pym/portage/emaint/modules/binhost/binhost.py
> b/pym/portage/emaint/modules/binhost/binhost.py
> index cf1213e..8cf3da6 100644
> --- a/pym/portage/emaint/modules/binhost/binhost.py
> +++ b/pym/portage/emaint/modules/binhost/binhost.py
> @@ -86,7 +86,9 @@ class BinhostHandler(object):
>  		stale = set(metadata).difference(cpv_all)
>  		for cpv in stale:
>  			errors.append("'%s' is not in the
> repository" % cpv)
> -		return errors
> +		if errors:
> +			return (1, errors)
> +		return (os.EX_OK, None)
>

Alexandru, I tend to not prefer using numbers to indicate success/fail.
They can be confusing at times.  In this case 1 means fail, 0 means
success.  Which is the opposite of True/False.  It would be different
if there were multiple return code values.  Then they would be
meaningful.  I want to keep these modules pythonic in the sense that
they return standard True/False for API use.  In this case returning
True/False would also make it easy to process the list of rcodes
returned to main()

sys.exit(False in rcodes)

which will reverse the True/False to 0/1 automatically for us and make
it super simple to process a variable length list success/fail results.

-- 
Brian Dolbec <dolsen>