Skip to content

[JENKINS-67978] Fix for job parameters on built-in node#232

Merged
basil merged 1 commit intojenkinsci:masterfrom
nvsmirnov:bugfix/JENKINS-67978
Oct 9, 2024
Merged

[JENKINS-67978] Fix for job parameters on built-in node#232
basil merged 1 commit intojenkinsci:masterfrom
nvsmirnov:bugfix/JENKINS-67978

Conversation

@nvsmirnov
Copy link
Copy Markdown
Contributor

@nvsmirnov nvsmirnov commented Jun 19, 2023

This PR fixes problem when option "Prevent multiple jobs with identical parameters from running concurrently" is not working (at least one issue is open: JENKINS-67978), and I've seen some similar question on the Internet.

Testing done

I tested this change manually in my environment. Before applying this patch, parameters were ignored, same job with equal parameters was triggered, didn't wait for the previous job with the same parameter to finish.
After applying the patch, job with the same paparameters is waiting for an existing job with the same parameters to finish - exactly as it shoud..

I am not a programmer, so I'm not able to implement automated tests. I was able to add some debugging and to guess what may fix the problem, but this is already too good for me :-)

Note there are few other places where getExecutors() is used in this plugin, may be someone need to consider change that paces too, or may be not, I'm not sure. I suggest to approve this PR first because it fixes the problem that really needed by some users, and then decide if other places needs to be changed similarly.

P.S. There is some kind of explanation on my findings in my first comment to JENKINS-67978

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

Closes #269

@nvsmirnov nvsmirnov requested a review from a team as a code owner June 19, 2023 14:16
@basil
Copy link
Copy Markdown
Member

basil commented Oct 7, 2024

I am not really maintaining this plugin anymore, so feel free to take over:

https://www.jenkins.io/doc/developer/plugin-governance/adopt-a-plugin/

Copy link
Copy Markdown
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@basil basil changed the title Fix for job parameters on built-in node [JENKINS-67978] Fix for job parameters on built-in node Oct 9, 2024
@basil basil added the bug label Oct 9, 2024
@basil basil merged commit ef682b2 into jenkinsci:master Oct 9, 2024
@nvsmirnov
Copy link
Copy Markdown
Contributor Author

Hello @basil!

Is it possible to set the required Jenkins version to 2.477 instead of 2.479?
The next LTS release will be 2.477, and those, who are on LTS, will not be able to upgrade to new plugin version for other three months...

@basil
Copy link
Copy Markdown
Member

basil commented Oct 15, 2024

The next LTS release will be 2.479.1 which is scheduled for release on October 30 (in two weeks).

@nvsmirnov
Copy link
Copy Markdown
Contributor Author

nvsmirnov commented Oct 15, 2024

Thanks for prompt reply @basil :-)

But will it really be 2.479?
I'm asking because I looked to event calendar (this specific event)

And it says that it will be 2.477.1

@basil
Copy link
Copy Markdown
Member

basil commented Oct 15, 2024

The calendar is just out of date. @MarkEWaite (in his role as release lead) can update it I think.

@MarkEWaite
Copy link
Copy Markdown
Contributor

Thanks for reporting that @nvsmirnov and thanks for the mention @basil . I've updated the Jenkins calendar to show the correct LTS baseline version is 2.479, not 2.477.

@nvsmirnov
Copy link
Copy Markdown
Contributor Author

Ok, thanks a lot guys! :-)

@basil
Copy link
Copy Markdown
Member

basil commented Nov 18, 2024

@nvsmirnov Is this working well in production?

@nvsmirnov
Copy link
Copy Markdown
Contributor Author

@basil yes, I checked it in my environment, it works, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants