Skip to content

Fix race condition in JenkinsTriggerHelper - #229

Merged
khmarbaise merged 3 commits into
jenkinsci:masterfrom
Veske:master
Feb 3, 2017
Merged

Fix race condition in JenkinsTriggerHelper#229
khmarbaise merged 3 commits into
jenkinsci:masterfrom
Veske:master

Conversation

@Veske

@VeskeVeske commented Feb 3, 2017

Copy link
Copy Markdown
Contributor

Fix race condition in JenkinsTriggerHelper

@VeskeVeske closed this Feb 3, 2017
@VeskeVeske reopened this Feb 3, 2017
@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

I have checked the build it's not your fault the failing build....

@Veske

Veske commented Feb 3, 2017

Copy link
Copy Markdown
ContributorAuthor

Alright, I don't have the options currently to set-up the environment to run your IT tests also. Will wait till you guys can figure it out then.

@khmarbaise

khmarbaise commented Feb 3, 2017•
edited
Loading

Copy link
Copy Markdown
Member

So the problem was a fix which failed an integration tests which was an wrong implementation...which is now fixed...Can you please rebase against current master...

khmarbaiseand others added 2 commits February 3, 2017 22:46
@khmarbaiseVeske
Fixedjenkinsci#220Followup
984d828
 o getViews() default api/json?depth=1 cause timeout
but that change has caused in a failing integration
test case which is now fixed by using tree=.. parameters.
Kaspar Tint
Simplify the triggerJobAndWaitUntilFinished method a bit
628dd1a
Make the polling rate configurable
@Veske

Veske commented Feb 3, 2017•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

I might have messed up the PR, includes your changes also. Can you check if it's okay? Also if you could advice me on how to run these IT tests then I could maybe write an IT for this also later. Right now I wrote it "blindly".

@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

You can the IT's simply by using: mvn -Prun-docker-its clean verify But you need to start the docker container in the sub project jenkins-client-it-docker first (and run the script build-docker.sh first and afterwards start-docker.sh) ...You need of course Docker(version 1.12 recommended) installed on your machine...

BTW: Currently I'm trying to make that more simpler, cause I find it too complex as well...But the travis build already says everything looks ok...

I assume you did something wrong while you tried to do the rebase...
Simply can be done in your branch:
git rebase master

This will integrate all changes from the master which have been committed after you have created your branch...in the end a git log -1 should show only your single commit...you have made..Afterwards you need to do a git push --force origin BRANCH which will push that back...

@khmarbaise
khmarbaise merged commit a1ef2d0 into jenkinsci:masterFeb 3, 2017
@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

Looks fine so far, cause I got only a fast-forward pull...No problem.

@khmarbaisekhmarbaise added this to the Release 0.3.8 milestone Feb 3, 2017
@Veske

Veske commented Feb 3, 2017

Copy link
Copy Markdown
ContributorAuthor

I mean the method it self, that it works. Not at work currently, have nothing to test against. Did look ok tough.

@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

Updated the ReleaseNotes with a hint to your GitHub reference. Thanks for your pull request.

@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

Can you now close the PR...

@Veske

Veske commented Feb 3, 2017

Copy link
Copy Markdown
ContributorAuthor

Umh... It's closed now, no?

@khmarbaise

khmarbaise commented Feb 3, 2017

Copy link
Copy Markdown
Member

Ah sorry..missed that it is already closed. Sorry..

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

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

Release 0.3.8

Development

Successfully merging this pull request may close these issues.

2 participants

@Veske@khmarbaise