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
make sure scratch build from PR finished #193
Conversation
a55e0f4
to
f0aea42
Compare
f0aea42
to
f620ed2
Compare
@thrix could you check this? |
popd | ||
|
||
SCRATCHID=$(cat ${LOGDIR}/kojioutput.txt | awk '/Created task:/ { print $3 }') | ||
echo "koji_task_id=${SCRATCHID}" >> ${LOGDIR}/job.props | ||
|
||
# Make sure koji build finished | ||
# https://pagure.io/fedora-ci/general/issue/76 | ||
while koji taskinfo ${SCRATCHID} | grep "State: open"; do |
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.
this (taskinfo) could have a temporary error that leads to grep failure. also, the status could be "free". should this grep the other way around (i.e. continue to loop if it cannot grep closed|failed|cancelled?
f620ed2
to
709acd9
Compare
popd | ||
|
||
SCRATCHID=$(cat ${LOGDIR}/kojioutput.txt | awk '/Created task:/ { print $3 }') | ||
echo "koji_task_id=${SCRATCHID}" >> ${LOGDIR}/job.props | ||
|
||
# Make sure koji build finished | ||
# https://pagure.io/fedora-ci/general/issue/76 | ||
while koji taskinfo ${SCRATCHID} | grep "State:" | grep -Ev "closed|failed|cancelled"; do |
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.
when this greps failed...
fi | ||
done | ||
|
||
echo "status=SUCCESS" >> ${LOGDIR}/job.props |
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.
...it jumps here and sets the result to success.
709acd9
to
d1bd49e
Compare
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 haven't tested the code, but the logic seems legit now.
@bgoncalv seems legit to me also. so +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.
LGTM
[merge] |
@hroncok thanks for the review :) |
The following image promotions have taken place:
|
related to https://pagure.io/fedora-ci/general/issue/76