Wow, serendipity. I was going to implement that later tonight :)

Nice one!

Just a couple of comments inline:

On 23/05/2011, at 6:02 PM, [email protected] wrote:

> 
> [MDEPLOY-133]

I think the log comment could cover more :)
> 
> +            catch ( ArtifactDeploymentException e )
> +            {

Are there specific types of exceptions that are "fatal" that can be captured? 
502/503 responses make sense to retry, but not sure about 500, 403, 401, etc.

> +                if (count + 1 < retryFailedDeploymentCount) {
> +                    getLog().warn( "Something went wrong with the 
> deployment, will try again", e );

This may be more verbose than necessary - how about just e.getMessage, with the 
trace logged at debug level?

> +                }
> +                if ( exception == null )
> +                {
> +                    exception = e;
> +                }

A completely new exception that says to refer to the other failures might be 
more appropriate, if they happen to differ... though that may be too much of an 
edge case :)

Cheers,
Brett

--
Brett Porter
[email protected]
http://brettporter.wordpress.com/
http://au.linkedin.com/in/brettporter





---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to