Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-16634] Do not fail to write a log file just because something deleted the parent directory #2738

Conversation

jglick
Copy link
Member

@jglick jglick commented Feb 6, 2017

JENKINS-16634

No one is sure why the directory is deleted, but refusing to connect an agent because of this is not graceful. Test reproduced a similar error.

@reviewbybees

@@ -772,6 +772,7 @@ THE SOFTWARE.
<forkCount>0.5C</forkCount>
<reuseForks>true</reuseForks>
<argLine>-XX:MaxPermSize=128m -noverify</argLine> <!-- some versions of JDK7/8 causes VerifyError during mock tests: http://code.google.com/p/powermock/issues/detail?id=504 -->
<trimStackTrace>false</trimStackTrace> <!-- SUREFIRE-1226 workaround -->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise the pre-fix test failure omits the interesting bits in its stack trace.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but Ok. Woukd be better to have it in a separate PR

* @author Kohsuke Kawaguchi
*/
public class ReopenableRotatingFileOutputStreamTest {
public class RewindableRotatingFileOutputStreamTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReopenableRotatingFileOutputStream was deprecated in #2490 and SlaveComputer now uses RewindableRotatingFileOutputStream, so test that.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐝

@@ -772,6 +772,7 @@ THE SOFTWARE.
<forkCount>0.5C</forkCount>
<reuseForks>true</reuseForks>
<argLine>-XX:MaxPermSize=128m -noverify</argLine> <!-- some versions of JDK7/8 causes VerifyError during mock tests: http://code.google.com/p/powermock/issues/detail?id=504 -->
<trimStackTrace>false</trimStackTrace> <!-- SUREFIRE-1226 workaround -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but Ok. Woukd be better to have it in a separate PR

@jglick jglick added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Feb 7, 2017
@jglick
Copy link
Member Author

jglick commented Feb 7, 2017

@reviewbybees done

@daniel-beck daniel-beck merged commit 3f235e0 into jenkinsci:master Feb 8, 2017
daniel-beck added a commit that referenced this pull request Feb 13, 2017
@jglick jglick deleted the RewindableRotatingFileOutputStream-JENKINS-16634 branch February 14, 2017 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants