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-37219] Add a job property for overriding branch index triggers #48

Merged
merged 5 commits into from Aug 28, 2016

Conversation

Projects
None yet
5 participants
@abayer
Copy link
Member

commented Aug 17, 2016

JENKINS-37219

Works in both directions - disables branch index triggers if set to
false, enables them if set to true and either there's a
NoTriggerBranchProperty applying to the branch or there's a
NoTriggerOrganizationFolderProperty applying to the organization.

Theoretically, the config shouldn't show up for any job type but
multibranch jobs, where it'll only show up in the Snippet Generator anyway.

jenkinsci/workflow-multibranch-plugin#26 will be replaced with an integration test of this in the properties step.

cc @reviewbybees esp @jglick

[FIXED JENKINS-37219] Add a job property for overriding branch index …
…triggers.

Works in both directions - disables branch index triggers if set to
false, enables them if set to true and either there's a
NoTriggerBranchProperty applying to the branch or there's a
NoTriggerOrganizationFolderProperty applying to the organization.

Theoretically, the config shouldn't show up for any job type but
multibranch jobs, where it'll only show up in the Snippet Generator anyway.
@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2016

So the config properly doesn't show up normally! The problem is it only shows up with the help and nothing else in multibranch snippet generator. D'oh. Working on it.

@reviewbybees

This comment has been minimized.

Copy link

commented Aug 17, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

Fixed config decision making, but it's still not showing up for snippet.
I can't see a way to tell whether we're being called for a Snippet
Generator from a multibranch job vs a non-multibranch job...
@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2016

@jglick So...how would I distinguish between when the snippet generator is for a multibranch job rather than for a non-multibranch job? 'cos otherwise, it just doesn't show up in the snippet generator ever.

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2016

ping @jglick

@Symbol("overrideIndexTriggers")
public static class DescriptorImpl extends JobPropertyDescriptor {

public boolean isOwnerMultibranch(Job<?,?> owner) {

This comment has been minimized.

Copy link
@jglick

jglick Aug 23, 2016

Member

it could be any Item (cf. Snippetizer/index.jelly and getItem(StaplerRequest)). To account for Pipeline Syntax (i.e., Snippetizer) being displayed from various contexts—OrganizationFolder, MultiBranchProject, WorkflowJob—you could perhaps check

return item instanceof MultiBranchProject || item instanceof OrganizationFolder || item.getParent() instanceof MultiBranchProject;

Try something along those lines.

abayer added some commits Aug 23, 2016

@jglick jglick changed the title [FIXED JENKINS-37219] Add a job property for overriding branch index … [FIXED JENKINS-37219] Add a job property for overriding branch index triggers Aug 26, 2016

@jglick jglick changed the title [FIXED JENKINS-37219] Add a job property for overriding branch index triggers [JENKINS-37219] Add a job property for overriding branch index triggers Aug 26, 2016

pom.xml Outdated
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.4-SNAPSHOT</version>

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐛 update to 1.4

@@ -73,11 +73,17 @@ public boolean shouldSchedule(Queue.Task p, List<Action> actions) {
if (p instanceof Job) {
Job j = (Job) p;
if (j.getParent() instanceof MultiBranchProject) {

OverrideIndexTriggersJobProperty overrideProp =
(OverrideIndexTriggersJobProperty) j.getProperty(OverrideIndexTriggersJobProperty.class);

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

BTW if you declare j to be a Job<?,?> you do not need the cast.

for (BranchProperty prop : ((MultiBranchProject) j.getParent()).getProjectFactory().getBranch(j).getProperties()) {
if (prop instanceof NoTriggerBranchProperty) {
if (prop instanceof NoTriggerBranchProperty
&& (overrideProp == null || !overrideProp.getEnableTriggers())) {

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 easier, clearer, and (ever so slightly) faster to write

OverrideIndexTriggersJobProperty overrideProp = j.getProperty(OverrideIndexTriggersJobProperty.class);
if (overrideProp != null) {
    return overrideProp.getEnableTriggers();
} else {
    for (BranchProperty prop : ((MultiBranchProject) j.getParent()).getProjectFactory().getBranch(j).getProperties()) {
        if (prop instanceof NoTriggerBranchProperty) {
            return false;
        }
    }
}
@@ -90,14 +90,18 @@ public boolean shouldSchedule(Queue.Task p, List<Action> actions) {
if (c instanceof BranchIndexingCause) {
if (p instanceof Job) {
Job j = (Job) p;
OverrideIndexTriggersJobProperty overrideProp =
(OverrideIndexTriggersJobProperty) j.getProperty(OverrideIndexTriggersJobProperty.class);

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 ditto

if (j.getParent() instanceof MultiBranchProject) {
MultiBranchProject mbp = (MultiBranchProject) j.getParent();
if (mbp.getParent() instanceof OrganizationFolder) {
NoTriggerOrganizationFolderProperty prop = ((OrganizationFolder) mbp.getParent()).getProperties().get(NoTriggerOrganizationFolderProperty.class);
if (prop != null) {
// Not necessarily the same as j.getName(), which may be encoded:
String name = mbp.getProjectFactory().getBranch(j).getName();
if (!name.matches(prop.getBranches())) {
if (!name.matches(prop.getBranches()) &&
(overrideProp == null || !overrideProp.getEnableTriggers())) {

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 ditto

* suppress them, or disabling if they're otherwise enabled.
*/
public class OverrideIndexTriggersJobProperty extends JobProperty<Job<?,?>> {
private boolean enableTriggers;

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 final

if (p instanceof Job) {
Job j = (Job) p;
OverrideIndexTriggersJobProperty overrideProp =
(OverrideIndexTriggersJobProperty) j.getProperty(OverrideIndexTriggersJobProperty.class);

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 ditto

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form">
<j:if test="${descriptor.isOwnerMultibranch(it)}">
<f:block>

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐛 I think you need to delete the f:block wrapper. Configuration forms are generally in the context of a rowset; f:optionalBlock expects to be in a rowset and invokes its body in a rowset; f:block expects to be in a rowset and invokes its body as inline content.

Even if a particular browser is forgiving, HtmlUnit etc. may not be.

This comment has been minimized.

Copy link
@abayer

abayer Aug 26, 2016

Author Member

Ok.

@@ -0,0 +1,11 @@
<div>

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

You might want to make this help-enableTriggers.html. Not sure which looks better in practice.

This comment has been minimized.

Copy link
@abayer

abayer Aug 26, 2016

Author Member

Something went weird when I did it as help-enableTriggers.html, but can't remember what off the top of my head.

@Rule
public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
@Rule
public GitSampleRepoRule orgSampleRepo = new GitSampleRepoRule();

This comment has been minimized.

Copy link
@jglick

jglick Aug 26, 2016

Member

🐜 Why the two rules? You do not appear to be using both from a single test case.

This comment has been minimized.

Copy link
@abayer

abayer Aug 26, 2016

Author Member

...because I forgot they reset between test cases.

@jglick

This comment has been minimized.

Copy link
Member

commented Aug 26, 2016

🐝

@abayer abayer merged commit 7eb7644 into jenkinsci:master Aug 28, 2016

1 check passed

Jenkins This pull request looks good
Details
@e0003315

This comment has been minimized.

Copy link

commented Jun 13, 2018

Hi, i am new to jenkins pipeline. Do you have an example of how i can use this to prevent automatic scm trigger in jenkinsfile? it looks complicated. I was hoping for something like properties[autoSCMTrigger: false]. I know you can set under configurations but i am using cloudbees jenkins now.

@stephenc

This comment has been minimized.

Copy link
Member

commented Jun 13, 2018

This is not the way to do that. This was a hack that has now been superseded. What you want to do is add a branch build strategy plugin (for example https://plugins.jenkins.io/basic-branch-build-strategies ) then you can define for a multibranch project which branches should trigger a build when a change is detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.