-
Notifications
You must be signed in to change notification settings - Fork 112
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
add support for limiting job concurrency based on one or more build parameters #9
base: master
Are you sure you want to change the base?
Conversation
…parameters basic support for limiting concurrent job runs by parameters
Selectable parameters
…kins config reload.
plugins » throttle-concurrent-builds-plugin #6 FAILURE |
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
@@ -26,13 +26,13 @@ THE SOFTWARE. | |||
<parent> | |||
<groupId>org.jenkins-ci.plugins</groupId> | |||
<artifactId>plugin</artifactId> | |||
<version>1.399</version> | |||
<version>1.518</version> |
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.
Could you reduce version dependency to 1.480.x at least? Old LTS releases still alive...
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.
Yes, I'll try that.
On Mon, Nov 4, 2013 at 12:20 PM, Oleg Nenashev [email protected]:
In pom.xml:
@@ -26,13 +26,13 @@ THE SOFTWARE.
org.jenkins-ci.plugins
plugin
- 1.399
- 1.518
Could you reduce version dependency to 1.480.x at least? Old LTS releases
still alive...—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9/files#r7412031
.
plugins » throttle-concurrent-builds-plugin #17 FAILURE |
plugins » throttle-concurrent-builds-plugin #18 UNSTABLE |
plugins » throttle-concurrent-builds-plugin #23 UNSTABLE |
plugins » throttle-concurrent-builds-plugin #28 SUCCESS |
this should be good to go now. |
@oleg-nenashev do you have access to review and merge this PR? Thanks |
@joemiller , @Hornswoggles From my point of view it makes sense to create a |
@@ -26,7 +26,7 @@ THE SOFTWARE. | |||
<parent> | |||
<groupId>org.jenkins-ci.plugins</groupId> | |||
<artifactId>plugin</artifactId> | |||
<version>1.424</version> | |||
<version>1.509.4</version> |
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.
Do we really need to change the version? I don't see real dependencies from Jenkins core.
I also recommend to avoid 1.509.4 at least. See https://groups.google.com/forum/#!topic/jenkinsci-dev/9fMrUsVh6zU
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.
I am not sure, possibly. It's been a while since we started this patch and
I don't recall if we bumped version for a reason.
Would the proper approach to testing this be to simply revert the version
in the pom.xml and run the test suite?
thanks
On Fri, Apr 4, 2014 at 10:46 AM, Oleg Nenashev [email protected]:
In pom.xml:
@@ -26,7 +26,7 @@ THE SOFTWARE.
org.jenkins-ci.plugins
plugin
- 1.424
- 1.509.4
Do we really need to change the version? I don't see real dependencies
from Jenkins core.
I also recommend to avoid 1.509.4 at least. See
https://groups.google.com/forum/#!topic/jenkinsci-dev/9fMrUsVh6zUReply to this email directly or view it on GitHubhttps://github.com//pull/9/files#r11305151
.
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.
Test suites don't provide enough test coverage. BTW, it is better than nothing
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.
Throttling by parameter doesnt work on 1.424 I will go through each LTS release between 1.424 and 1.509. Starting with 1.424.6
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.
LTS 1.447 builds and works. Although the helpful message does not show up in the queue
LTS 1.466 builds and works and has the helpful message in the queue
Do you plan to merge those changes about controlling throttleling based on build parameters? |
I merge the current master branch into this pull request and tried to follow the review comments in this pull request. These I do not know how to implement:
If you give me more specific guidance how to fix these, I will try to implement them also. Here is the repository: https://github.com/jmozmoz/throttle-concurrent-builds-plugin |
Ping |
Which performance issues exactly have to be solved? I can't find any of them being discussed in comments on this pull request. |
As requested a I move the list paramsToCompare to a transient field which is filled by an overwritten load()/save()? Is this the way to go? |
Yes, it may work. I've lost the track of this PR a bit, need to review it again. Did you have a chance to process the previous comments? |
I created a new pull request #33 including upstream changes not merged into in this original pull request. Also I think I addressed the following issues (striked out):
|
Now the pull request #33 builds but there are some test failures. I have no idea, how these test failures are related to the work here. Could somebody please help. |
Can this PR be closed since #38 has been merged? |
Submitting this PR to see if this feature set is something the upstream would be in favor of accepting. And if so, what changes or cleanup would be necessary to get it upstream?