[libvirt] [jenkins-ci PATCH] lcitool: Raise Error instead of Exception

[Andrea Bolognani]

On Mon, 2019-03-11 at 18:24 +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 11, 2019 at 07:11:49PM +0100, Andrea Bolognani wrote:
> > On Mon, 2019-03-11 at 17:55 +0000, Daniel P. Berrangé wrote:
> > > I'm curious why the "Error" class exists at all ? It doesn't seem
> > > to add anything that the normal "Exception" class can't do, and
> > > leads to bugs like the one here.
> > 
> > I seem to understand you're not supposed to use Exception directly,
> > but rather define your own exception types:
> > 
> >   https://docs.python.org/3/tutorial/errors.html#user-defined-exceptions
> > 
> > I remember reading more about this, but I can't find the source
> > right now.
> 
> I think it largely depends on the scope / scale of the application you
> are writing. If the code is complex enough that you're going to want
> to be catching exceptions in specific scenarios, then creating subclasses
> makes sense as you can avoid mistakenly catching too much exceptions.
> 
> In the lcitool case though, we're not trying to do anything very clever
> with catching specific exceptions as our app is quite small & self
> contained. We just want to catch everything to hide the ugly stack traces
> by default. IMHO in that scenario it is fine to just use the base exception
> class directly. BTW, there's no need for the "message" field, even with
> the Error class, as the default Exception class will stringify to report
> just the message it received: eg this is equiv:
> 
> @@ -703,5 +700,5 @@ if __name__ == "__main__":
>      try:
>          Application().run()
>      except Error as err:
> -        sys.stderr.write("{}: {}\n".format(sys.argv[0], err.message))
> +        sys.stderr.write("{}: {}\n".format(sys.argv[0], err))
>          sys.exit(1)
> 
> anyway, I think we can just simplify our life & delete the Error class.
> 
> It would probably be worth registering a "--debug" flag to the CLI to
> disable the global  exception catch, so that we can see the full stack
> trace when needed.

I don't have a problem with any of the changes you describe. I'm
far from being an expert at Python, so having used suboptimal
constructs when implementing lcitool doesn't surprise me one bit :)

-- 
Andrea Bolognani / Red Hat / Virtualization