-
Notifications
You must be signed in to change notification settings - Fork 87
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
skip passed work #3575
base: develop
Are you sure you want to change the base?
skip passed work #3575
Conversation
Jenkinsfile
Outdated
git diff | ||
git diff-index --quiet HEAD || (echo "Generated files are different. Please run make generate and commit the changes." && exit 1) | ||
make -j\$(nproc) all package check VERBOSE=1 | ||
#make -j\$(nproc) all package check VERBOSE=1 |
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.
We should skip the entire stage including docker/cmake builds and even possibly checkout. This should probably be done in the node(name)
section below, or before if possible since we are just checking a commit hash.
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.
Even better would be to do it in(or around) the Check image
stage. Set an env variable to a list of the contexes that are success
, and then we can check that variable before we even allocate a node. It would also reduce the number github API calls we need to make(it would be once per build instead of once per build per stage).
Jenkinsfile
Outdated
md5sum ./*.deb | ||
echo ${GIT_COMMIT_SHA} > "${name}_PR" |
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 dont think storing it locally in the workspace is useful since it wont exist on every node. Instead you should check the github commit status, something like this:
withCredentials([string(credentialsId: "${env.migraphx_ci_creds}", variable: 'GITHUB_TOKEN')]) {
def commit_hash = sh(script: 'git rev-parse --short HEAD', returnStdout: true).trim()
def response = sh(
script: """curl -H "Accept: application/vnd.github+json" \
-H "Authorization: token ${GITHUB_TOKEN}" \
https://api.github.com/repos/ROCm/AMDMIGraphX/commits/${commit_hash}/status""",
returnStdout: true
).trim()
def statuses = response.statuses
def contextStatus = statuses.find { it.context == "Jenkins - ${variant}"
env.COMMIT_PASSED = contextStatus != null && contextStatus.state == 'success'
}
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.
interesting, I was trying to use archieveAritfact since it would be shared across nodes, but I hadn't figured out a way to pull the file down. I like your idea and will try that
Check results before merge 🔆 |
🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
Still testing, but the idea is to skip a passed test if the current commit hash matches is marked as passed already