Brett Porter wrote:
Hi Joakim,
On 01/11/2007, at 9:47 AM, [EMAIL PROTECTED] wrote:
Fixing release / snapshot policies from applying tests on
maven-metadata.xml files.
Was this the addition of the filetype property - or something else as
well? Bit hard to find in the change :)
Yes. the filetype property.
Didn't want to pull in all of archiva-repository-layer just to get the
benefits of metadata detection.
Felt i could just let the proxy handlers determine scope (artifact /
metadata / supportfile) and pass it down the the policy handlers.
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.
getLogger().debug( "Applying [" + key + "] policy with
[" + setting + "]" );
- if ( !policy.applyPolicy( setting, request, localFile ) )
+ try
{
- getLogger().debug( "Didn't pass the [" + key + "]
policy." );
- return false;
+ policy.applyPolicy( setting, request, localFile );
+ }
+ catch ( PolicyConfigurationException e )
+ {
+ getLogger().error( e.getMessage(), e );
}
}
This is going to be very noisy if it's misconfigured - is there
anything that can test these on system configuration so we can fail fast?
As for noise, the standard failure reasoning will easily be
(10*(proxyConnector.size()) more noisy than the configuration errors.
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.
Work on MRM-549 is proceeding. Go comment on that other thread about
SKIP/REJECT.
The change to that config.load() is due to MRM-549, but I'm cleaning up
a ton of other proxy connector nonsense too.
--
- Joakim Erdfelt
[EMAIL PROTECTED]
Open Source Software (OSS) Developer
