-
Notifications
You must be signed in to change notification settings - Fork 69
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-71722][JENKINS-71758] Respect globally configured period for Periodic generation #485
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it would make the system property not work:
Lines 135 to 139 in 305a08c
public void setPeriod(Integer period) { | |
if (!isEnforcedPeriod()) { | |
this.period = Math.max(Math.min(24, period), 1); | |
} else { | |
this.period = SupportPlugin.AUTO_BUNDLE_PERIOD_HOURS; |
Lines 100 to 102 in 305a08c
public int getPeriod() { | |
return period; | |
} |
Right, testing this, the system property is not effective anymore. Maybe we should put the enforcement in the getter rather than the setter. |
I have also noticed inconsistencies in the UI JENKINS-71758. I think it is best to fix this as part of this PR too. |
if (json.getBoolean("enabled") && !json.has("components")) { | ||
|
||
if(isEnforcedDisabled()) { | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to suppress unrelated config like
Line 190 in 210c61f
setComponentIds(parseRequest(req, json)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It leaves the config untouched actually. I tested this.
-
If you set
-Dcom.cloudbees.jenkins.support.SupportPlugin.AUTO_BUNDLE_PERIOD_HOURS=0
and then go to the config page and save, it leaves the entire configuration as is (includingcom.cloudbees.jenkins.support.config.SupportAutomatedBundleConfiguration.xml
). So that if you then remove the system property, you are back to your original configuration that you had. -
If you set
-Dcom.cloudbees.jenkins.support.SupportPlugin.AUTO_BUNDLE_PERIOD_HOURS=2
and then go to the config page and save, it leaves only theperiod
configuration as is (includingcom.cloudbees.jenkins.support.config.SupportAutomatedBundleConfiguration.xml
). If you later remove the system property, the generation is still enabled and you are back to your originalperiod
value.
Does that makes sense ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whatever you say—I am not familiar with the details of this form.
JENKINS-71722: global configuration (disablement and period) was not respected in the periodic generation.
Testing done
Submitter checklist