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

Created a general function for retrying commands #7208

Merged
merged 1 commit into from Oct 4, 2019

Conversation

JennyChenyj
Copy link

[skip ci]

Signed-off-by: Jenny Chen Jenny.Chen@ibm.com

@JennyChenyj JennyChenyj changed the title Created a general function for retrying commands WIP Created a general function for retrying commands Sep 24, 2019
@JennyChenyj
Copy link
Author

Hi @AdamBrousseau. Thanks!

@JennyChenyj
Copy link
Author

Hi @AdamBrousseau! The order has been changed. Thanks!

} else {
ret = true
}
command()
Copy link
Contributor

Choose a reason for hiding this comment

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

Indenting needs to be fixed

@@ -606,6 +606,18 @@ def generate_test_jobs(TARGET_NAMES, SPEC, ARTIFACTORY_SERVER, ARTIFACTORY_REPO)
}
}

def retry_and_delay(command, numRetries = 3, waitTime = 60, units = 'SECONDS') {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a comment about how to use this function.

def ret = false
retry(numRetries) {
if (ret) {
sleep time: waitTime,unit: units
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitepsace after comma.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for find those mistakes! I have fixed them. Thanks for checking it again~

@JennyChenyj JennyChenyj force-pushed the AddRetryWait branch 3 times, most recently from 8578e6d to bf8f94f Compare September 24, 2019 18:40
Copy link
Contributor

@AdamBrousseau AdamBrousseau left a comment

Choose a reason for hiding this comment

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

Looks good. Once #7110 and #7145 are merged we can replace those retries with a call to this function.

@pshipton
Copy link
Member

They are merged now.

@JennyChenyj
Copy link
Author

They are merged now.

Thanks!

@JennyChenyj JennyChenyj force-pushed the AddRetryWait branch 2 times, most recently from 07fcdcb to e64db12 Compare September 25, 2019 16:55
@JennyChenyj
Copy link
Author

Hey @AdamBrousseau! I replaced both of them. Thanks for checking! :)

@AdamBrousseau
Copy link
Contributor

Jenkins compile plinux jdk11

@JennyChenyj
Copy link
Author

JennyChenyj commented Sep 25, 2019

Hey @AdamBrousseau! Sorry, I made a mistake of calling the function. I fixed it. Thanks!

@AdamBrousseau
Copy link
Contributor

Jenkins compile plinux jdk11

}
jobDsl targets: templatePath, ignoreExisting: false, additionalParameters: params
}
retry_and_delay({jobDsl targets: templatePath, ignoreExisting: false, additionalParameters: params}, 3, 120)
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll have to check if pipeline-functions.groovy is loaded at this point in the build and what variable it's loaded into. Same comment for build.groovy. Look for a load step
ex buildFile = load 'buildenv/jenkins/common/pipeline-functions.groovy'

Copy link
Author

Choose a reason for hiding this comment

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

Hey Adam! I think pipeline-functions.groovy is not loaded when it fails. I checked the console output and there is no such line
load 'buildenv/jenkins/common/pipeline-functions.groovy'. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, so we'll have to make sure we load that library in those files so we can utilize this new function.

@JennyChenyj JennyChenyj force-pushed the AddRetryWait branch 2 times, most recently from 555548e to 8e47549 Compare September 27, 2019 15:20
@JennyChenyj
Copy link
Author

Hey @AdamBrousseau! Thanks for checking it!

@AdamBrousseau
Copy link
Contributor

Jenkins compile plinux jdk11

@AdamBrousseau
Copy link
Contributor

Jenkins compile plinux jdk11

@AdamBrousseau
Copy link
Contributor

Jenkins compile plinux jdk11

Copy link
Contributor

@AdamBrousseau AdamBrousseau left a comment

Choose a reason for hiding this comment

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

LGTM. PR build tests the 2 function calls.

@pshipton pshipton changed the title WIP Created a general function for retrying commands Created a general function for retrying commands Sep 30, 2019
@pshipton pshipton self-assigned this Sep 30, 2019
@pshipton
Copy link
Member

The commits can be squashed, I don't think we need to have all three merged. Also, a different formatting for pipelineFunctions.retry_and_delay({server.upload spec: uploadSpec, buildInfo: buildInfo; server.publishBuildInfo buildInfo}, 3, 300) should be easier to read. Something like

pipelineFunctions.retry_and_delay({
    server.upload spec: uploadSpec, buildInfo: buildInfo;
    server.publishBuildInfo buildInfo},
    3, 300)

@JennyChenyj
Copy link
Author

The commits can be squashed, I don't think we need to have all three merged. Also, a different formatting for pipelineFunctions.retry_and_delay({server.upload spec: uploadSpec, buildInfo: buildInfo; server.publishBuildInfo buildInfo}, 3, 300) should be easier to read. Something like

pipelineFunctions.retry_and_delay({
    server.upload spec: uploadSpec, buildInfo: buildInfo;
    server.publishBuildInfo buildInfo},
    3, 300)

Thanks for the advice! I will make the change now.

@JennyChenyj JennyChenyj force-pushed the AddRetryWait branch 2 times, most recently from 44068b6 to acc6fb5 Compare September 30, 2019 18:52
Rewrote the retries for uploading to artifactory and creating jobs

[skip ci]

Signed-off-by: Jenny Chen <Jenny.Chen@ibm.com>
@JennyChenyj
Copy link
Author

Hi @pshipton! I have made the change. Thanks!

@pshipton pshipton merged commit ddbc3da into eclipse-openj9:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants