Creating a GlanceException with a specified msg containing kwargs does not work.

[Alex Meade]

unicode(GlanceException('test: %(code)s', code=500)) prints out "test: %(code)s" instead of "test 500"

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/8190

[Brian Waldon (bcwaldon)]

This isn't a bug - the GlanceException class is not designed to be used this way. What are you proposing we change the behavior to?

[Alex Meade (alex-meade)]

I was adding tests for glanceexceptions and it seemed inconsistent with how it was already working with kwargs that this did not work.

I also agree with what you said in the MP, we should not be tacking on "Details:..." in error messages.

[Brian Waldon (bcwaldon)]

Ok. So given that the design of GlanceException is to take a list of keyword args, why should it also accept positional args? If there's a good reason, I could be persuaded to conditionally replace the full '...unknown error...' string with what is passed in as the arg.

[Alex Meade (alex-meade)]

I don't think it should also take in positional args. I think if there is a unique message that needs to be in a glance exception then a new exception class should be made to represent that case.

Currently glance exception checks if there is a positional arg incase someone passed one in and just tacks in on to make sure no info is lost.

So I propose we just wipe out the "Details:..." stuff and only show the exception class message formatted with it's kwargs. That would really simplify this code and prevent misuse of GlanceExceptions, what do you think?

[Brian Waldon (bcwaldon)]

The purist in me definitely likes that much more, but somebody obviously added the Details crap for a reason. Can you do a git blame and figure out who/why that's there, and if we're going to cause any heartache if we remove it?

[Alex Meade (alex-meade)]

It was Jay Pipes...after further inspection, they're plenty cases where the exception message is being passed in as an arg.

Perhaps instead of tacking it on the end of the message, we should just make the message given as an arg the entire message. This is how it is done in nova. Or someone could go through and remove every case where it is being passed as an arg.

[Alex Meade (alex-meade)]

there are*

[Brian Waldon (bcwaldon)]

I think we just need to clean up the handling of positional args in GlanceException. Don't worry about fixing the cases where positional args are being used currently.

How would you propose handling multiple positional args or a mixture of positional and keyword args?

[Alex Meade (alex-meade)]

Hmm, I'll stew on this a bit and see if I can come up with something elegant

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/8190
Committed: http://github.com/openstack/glance/commit/e2c52ddfa6b5b6dff88e3d58b55ef7034739bc63
Submitter: Jenkins
Branch: master

commit e2c52ddfa6b5b6dff88e3d58b55ef7034739bc63
Author: Alex Meade <email address hidden>
Date: Tue Jun 5 13:52:23 2012 -0400

    Add tests and simplify GlanceExceptions.

    This also fixes some incorrect use of GlanceException parameters.

    Fixes bug: 1009122
    Fixes bug: 1010140

    Change-Id: Ic77856686a0259da2318e05b15f3d382508c00b9