Integration with coqbot #85
Conversation
two_points_on_the_same_branch.sh
Outdated
| # TODO: can we set the title automatically, rather than just echo this? | ||
| # e.g., cf https://ci.inria.fr/coq/cli/command/set-build-display-name, | ||
| # java -jar jenkins-cli.jar -s "${JENKINS_URL}" set-build-display-name "${JOB_NAME}" "${BUILD_NUMBER}" "PR #${coq_pr_number}: ${coq_pr_title}" | ||
| echo "Build for PR #${coq_pr_number}: ${coq_pr_title}" |
There was a problem hiding this comment.
Do we have access to the cli jar, or some way of doing this?
two_points_on_the_same_branch.sh
Outdated
| # if there's a comment id, we update the comment while we're | ||
| # in progress; otherwise, we wait until the end to post a new | ||
| # comment | ||
| if [ ! -z "${coq_pr_comment_id}" ]; then | ||
| # XXX TODO Is this the right way to get the data out there? Do we need to encode it? | ||
| curl -X POST --data-urlencode "${comment_text}" "${coqbot_url_prefix}/${coq_pr_number}/update-comment/${coq_pr_comment_id}" | ||
| elif [ ! -z "${is_done}" ]; then | ||
| # XXX TODO Is this the right way to get the data out there? Do we need to encode it? | ||
| curl -X POST --data-urlencode "${comment_text}" "${coqbot_url_prefix}/${coq_pr_number}" | ||
| fi | ||
| if [ ! -z "${is_done}" ]; then | ||
| # XXX TODO FIXME How should we communicate this to coqbot? | ||
| curl -X POST "${coqbot_url_prefix}/${coq_pr_number}/benchmarking-done" | ||
| fi |
There was a problem hiding this comment.
@Zimmi48 How should we be indicating these things to coqbot?
There was a problem hiding this comment.
I'd prefer to keep using static endpoints and put any additional information in the body of the request.
There was a problem hiding this comment.
What should I put into the body to indicate this?
There was a problem hiding this comment.
I am not sure I understood correctly your question. But the URL should be something static, like /benchmarking-done and the body should contain the information you need to pass to @coqbot. This could be the PR number on the first line, then the content to copy into the PR on the following lines. Anything that can be parsed easily is fine. JSON would be OK as well but it's probably not a good idea to generate JSON without a specialized tool.
There was a problem hiding this comment.
@Zimmi48 I've updated the urls to be static (I've used /update-comment, /new-comment, and /benchmarking-done to indicate that the needs: benchmarking label should be removed (there's a question of how to handle cases where the bench dies in the middle; do we want to leave needs: benchmarking in those cases, or do we want it removed when the bench is queued?) I'm currently having the text be url-encoded, so I think you can just split on & and then split each entry on the first = and then url-un-encode the contents.
There was a problem hiding this comment.
I'm currently having the text be url-encoded, so I think you can just split on
&and then split each entry on the first=and then url-un-encode the contents.
Hum, more work for me then.
There was a problem hiding this comment.
Feel free to suggest any other encoding that supports newlines, etc. This was the most common one that I saw used on the internet. If you just want the text raw, separated by newlines, I can also make it do that.
There was a problem hiding this comment.
Yep, I prefer to have the data in the either JSON format, or raw and newline separated.
There was a problem hiding this comment.
@Zimmi48 I've updated it to use
# Tell coqbot to update the in-progress comment
curl -X POST --data-binary "${coq_pr_number}${nl}${coq_pr_comment_id}${nl}${comment_text}" "${coqbot_url_prefix}/update-comment"# Tell coqbot to post a new comment that we're done benchmarking
curl -X POST --data-binary "${coq_pr_number}${nl}${comment_text}" "${coqbot_url_prefix}/new-comment"and
# Tell coqbot to remove the `needs: benchmarking` label
curl -X POST --data-binary "${coq_pr_number}" "${coqbot_url_prefix}/benchmarking-done"Does this work?
JasonGross
force-pushed
the
auto-bench
branch
2 times, most recently
from
d4d42ae
to
c131059
Compare
JasonGross
force-pushed
the
auto-bench
branch
2 times, most recently
from
820ac7b
to
980d32d
Compare
|
Now that pendulum has jq, should we merge this? I've updated the build configuration, so I think merging this won't break anything. Then someone needs to write the coqbot infrastructure to deal with this. |
JasonGross
changed the title
|
|
||
| # cf https://wiki.jenkins.io/display/JENKINS/Jenkins+CLI | ||
| # TODO: Is there a better place to put the cli? Is it already available somewhere? | ||
| curl "${JENKINS_URL}/jnlpJars/jenkins-cli.jar" -o "$working_dir/jenkins-cli.jar" |
There was a problem hiding this comment.
@maximedenes Is this (https://ci.inria.fr/coq/jnlpJars/jenkins-cli.jar) the same as the one that you pointed at that's at /home/jenkins/jenkins-slave/slave.jar
|
@ejgallego Can you review/approve/merge this? (Or give me permissions?) I believe it's ready for merge. |
@JasonGross I gave you permission [in a kind of hacky way, I need to work more on the setup] Anyways I'll merge myself as not to delay this more, thanks for the work! |
This reverts commit 487f705.
Closes #84
Closes coq/bot#8
Before this can be merged, I need to add
coq_pr_numberandcoq_pr_comment_idfields to the parameterized build. I'm waiting to do this until I get reviews on / indications of support for this PR.@Zimmi48 please advise on how to talk to coqbot appropriately
For integration in the other direction (step 2 of coq/bot#8 (comment)), we need to check off "Trigger builds remotely (e.g., from scripts)" under "Build Triggers", generate an "Authentication Token" and then have coqbot post(?) to
https://ci.inria.fr/coq/view/opam/job/benchmark-part-of-the-branch/buildWithParameters?token=$TOKEN&cause=coqbot&coq_pr_number=$PR_NUMBER&coq_pr_comment_id=$COMMENT_ID, I believe.