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-26582] Ignore attempts to schedule a MatrixConfiguration directly #19

Merged
merged 2 commits into from May 22, 2015

Conversation

jglick
Copy link
Member

@jglick jglick commented May 13, 2015

JENKINS-26582

@reviewbybees

}
}

if (a == null) {
LOGGER.log(Level.WARNING, "JENKINS-26582: ignoring apparent attempt to trigger {0} without its parent", getFullName());
Copy link
Member

Choose a reason for hiding this comment

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

I think Jenkins log is no a right place for JENKINS issue references. A comment would be better

Copy link
Member Author

Choose a reason for hiding this comment

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

There are various precedents for this. It is useful to have the issue noted in the Jenkins log, so that if you are trying to diagnose a warning you can immediately get the proper context.

Copy link
Member

Choose a reason for hiding this comment

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

@jglick Would a stack trace be helpful here, to see how this got scheduled?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think a stack trace would only indicate when this went to an executor, not when it was scheduled to the queue, but I could be wrong. If you rerun the test case for this bug with a Thread.dumpStack() here and it shows the buggy part of git-plugin then by all means add a 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.

@jglick I think you're right, those stack traces start in the executor, so wouldn't help.

@oleg-nenashev
Copy link
Member

@jglick
Looks good to me. I'm quite aware about corner cases when somebody launches a rebuild w/o a parent due to any reason, but I would consider it as a bug in other plugin. I've investigated Naginator and Matrix-Reloaded, and they submit builds properly..

@jglick
Copy link
Member Author

jglick commented May 14, 2015

I would consider it as a bug in other plugin.

Debatable. matrix-project really bends the rules and it is unclear to me who is at fault for such a trigger. I wonder if MatrixConfiguration should override isBuildable to return false, which would send the correct signal to other plugins (and ensure that scheduleBuild2 calls would get ignored). At any rate, that would be complementary to this patch, which addresses the problem at the lowest possible level: refusing to create a bogus MatrixRun.

@jglick jglick closed this May 14, 2015
@jglick jglick reopened this May 14, 2015
@jglick jglick closed this May 14, 2015
@jglick jglick reopened this May 14, 2015
@jglick
Copy link
Member Author

jglick commented May 21, 2015

@oleg-nenashev etc.?

@tfennelly
Copy link
Member

👍 insofar as I get it (can't see anything obviously wrong anyway)

@oleg-nenashev
Copy link
Member

👍

oleg-nenashev added a commit that referenced this pull request May 22, 2015
[JENKINS-26582] Ignore attempts to schedule a MatrixConfiguration directly
@oleg-nenashev oleg-nenashev merged commit b385c8a into jenkinsci:master May 22, 2015
@jglick jglick deleted the newBuild-JENKINS-26582 branch May 27, 2015 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants