Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/scm/SubversionSCM.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/scm/SubversionSCM.java

Kohsuke Kawaguchi
Administrator

I'm bit concerned about this change (and to some extent earlier change)
that disabled the project when an error is encountered.

There's a large possible cause for SVNException, ranging from
connectivity issue to authentication issue to failure to lock the
workspace. Not all of them are permanent problems, and for many of them
the build shouldn't be marked as disabled.

If we are to implement this feature, instead of checking for an
exception, we need to talk to Repository object and find out that the
path no longer exists (you can see a similar check implemented as
form-field validation in SubversionSCM.DescriptorImpl.)

Only when you can successfully confirmed that the directory no longer
exists (and also when the previous build run successfully from the same
location), you should mark the build as disabled.

For example, consider this scenario --- you are trying to set up your
project to check out from your SVN repo, but for some reason it's not
working, so you are tweaking configuration and doing trial&error. I
think you'd get quite annoyed if every attempt disables the job. You
might not even notice what it means for the job to be disabled if you
are new to Hudson.

So I think this kind of intelligence needs to be conservative.



[hidden email] wrote:

> User: jbq    
> Date: 2007-09-11 08:01:55+0000
> Modified:
>    hudson/hudson/main/core/src/main/java/hudson/scm/SubversionSCM.java
>
> Log:
>  Issue number:  763
>  
>  Disable project if an SVNException is thrown during polling.  In the future it
>  might be good to be more specific, ie recognize that the message in the cause
>  contains eg "Cannot replace a directory from within" to avoid disabling the
>  project when eg Internet is down.
>  
>  NOTE: the fix has not been tested with multiple repositories set (any hint about
>  what's the point of multiple repositories per project is welcome).
>  
>
> File Changes:
>
> Directory: /hudson/hudson/main/core/src/main/java/hudson/scm/
> =============================================================
>
> File [changed]: SubversionSCM.java
> Url: https://hudson.dev.java.net/source/browse/hudson/hudson/main/core/src/main/java/hudson/scm/SubversionSCM.java?r1=1.99&r2=1.100
> Delta lines:  +5 -2
> -------------------
> --- SubversionSCM.java 2007-09-11 05:34:47+0000 1.99
> +++ SubversionSCM.java 2007-09-11 08:01:53+0000 1.100
> @@ -615,7 +615,7 @@
>                  parseSvnInfo(SVNURL.parseURIDecoded(url), authProvider);
>              } catch (SVNException e) {
>                  // issue #763 Gracefully handle project deleted in SVN
> -                listener.getLogger().println("Project does not exist anymore in SVN: " + url);
> +                listener.getLogger().println("Project does not exist anymore in SVN: " + url + ", project will be disabled.");
>                  e.printStackTrace(listener.error(e.getMessage()));
>                  return true;
>              }
> @@ -645,7 +645,10 @@
>                      return true;    // change found
>                  }
>              } catch (SVNException e) {
> -                e.printStackTrace(listener.error("Failed to check repository revision for "+localInfo.getKey()));
> +                e.printStackTrace(listener.error("Failed to check repository revision for "+localInfo.getKey() + ", project will be disabled."));
> +                // Disable this project, see issue #763
> +                project.makeDisabled(true);
> +                return false;
>              }
>          }
>  
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/scm/SubversionSCM.java

Jean-Baptiste Quenot
* Kohsuke Kawaguchi:

> I'm bit concerned about this  change (and to some extent earlier
> change) that disabled the project when an error is encountered.
>
> There's a  large possible  cause for SVNException,  ranging from
> connectivity issue  to authentication  issue to failure  to lock
> the workspace. disabled.

Yes I agree, and realized this a few hours later.

> If  we  are  to  implement this  feature,  instead  of  checking
> for  an  exception,  we  need   to  talk  to  Repository  object
> and  find out  that  the  path no  longer  exists  (you can  see
> a  similar   check  implemented  as  form-field   validation  in
> SubversionSCM.DescriptorImpl.)

Thanks for  pointing this  out.  I  will make  this piece  of code
reusable.

Cheers,
--
     Jean-Baptiste Quenot
aka  John Banana   Qwerty
http://caraldi.com/jbq/

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