archiva-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brett Porter <>
Subject Re: svn commit: r590858 - in /maven/archiva/trunk/archiva-base: archiva-policies/src/main/java/org/apache/maven/archiva/policies/ archiva-policies/src/test/java/org/apache/maven/archiva/policies/ archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/
Date Wed, 31 Oct 2007 23:56:24 GMT

On 01/11/2007, at 10:46 AM, Joakim Erdfelt wrote:

>>> Switching from boolean return on .applyPolicy() to throwing  
>>> exception, to gain better logging of why the transfer failed.
>> No need to change it now, but I don't really agree with this. I  
>> don't agree with using exceptions for non-exceptional conditions  
>> for readability reasons - particularly just to pass a string  
>> around. IMO, better alternatives:
>> 1) just log in the policy check and return true/false as before
>> 2) return an object that has a "success" flag and a "reason"  
>> string (essentially the same as an exception, but correctly using  
>> the return type)
> The flaw with that is that only having a boolean doesn't tell us  
> WHY something failed.
> Also the log didn't make a distinction between config issue, normal  
> happy path, informational, and problem path.
> Since this is a failure, throw an exception.
> Made the code easier to read too.  Lots of uncoding here.

But that's what option 2 is for - roughly same amount of code, has a  
reason, but is still true/false return code. This isn't an  
exceptional condition - you would expect things to fail policies all  
the time when they are not updated, etc.

Anyway, it's just IMO, it obviously works just fine either way.

> As for noise, the standard failure reasoning will easily be (10* 
> (proxyConnector.size()) more noisy than the configuration errors.

but that's getting adjusted in a separate issue, right?

> Working on that now, the DefaultArchivaConfiguration.load()  
> mechanism has many configuration cleanup / sanity things in it.
> Shame you got rid of the ConfigurationCleanup interface a while  
> back. would have made this easy to test individually instead of all  
> lumped together like it is now.

I'm not really sure what you are referring to here, but let's focus  
on this some more after 1.0? I was just asking a question, I don't  
think we want to much with configuration any further at this point as  
the only issue I see open is one about clarity in the web interface.

- Brett

Brett Porter -

View raw message