Skip to content

Improve reporting of errors & efficiency of pr-size-labeler #7288

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

Merged
merged 16 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions .github/workflows/pr-labeler.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ run-name: >-
Label pull request ${{github.event.pull_request.number}} by ${{github.actor}}

on:
pull_request:
# Note: do not copy-paste this workflow with `pull_request_target` left as-is.
# Its use here is a special case where security implications are understood.
# Workflows should normally use `pull_request` instead.
pull_request_target:
types:
- opened
- synchronize
Expand All @@ -37,9 +40,9 @@ on:
description: 'The PR number of the PR to label:'
type: string
required: true
token:
description: 'Token to use for authentication:'
type: string
trace:
description: 'Turn on shell script debugging'
type: boolean

# Declare default permissions as read only.
permissions: read-all
Expand All @@ -55,23 +58,19 @@ jobs:
pull-requests: write
env:
PR_NUMBER: ${{inputs.pr-number || github.event.pull_request.number}}
# The next var is used by Bash. We add 'xtrace' to the options if this run
# is a workflow_dispatch invocation and the user set the 'trace' option.
SHELLOPTS: ${{inputs.trace && 'xtrace' || '' }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this used anywhere? If not please delete.

Copy link
Contributor Author

@mhucka mhucka Apr 29, 2025

Choose a reason for hiding this comment

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

Yes, it is. The environment variable is recognized by Bash: https://www.gnu.org/software/bash/manual/html_node/Bash-Variables.html#index-SHELLOPTS

Here what's happening is that when the Boolean trace is set by using workflow_dispatch, it causes SHELLOPTS to get the value xtrace. That's the same as using set -x: https://www.gnu.org/software/bash/manual/bash.html#index-set. Since the environment variable is set at the job level, all invocations get the SHELLOPTS value. The result is that all the Bash scripts and commands in the workflow get the equivalent of set -x, and thus output a trace of their steps. This is useful for debugging.

The beauty of this approach is that it doesn't require modifying any of the run: commands to add set -x, and it can be turned on and off at run time.

steps:
- name: Check out a copy of the git repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
sparse-checkout: |
./dev_tools/ci/size-labeler.sh

- name: Label the PR with a size label
id: label
continue-on-error: true
env:
GITHUB_TOKEN: ${{inputs.token || github.token}}
run: ./dev_tools/ci/size-labeler.sh |& tee action.out

- name: Detect and report errors in labeler action
if: steps.label.outcome == 'failure' || steps.label.outcome == 'error'
run: |
{
echo "<h4>‼️ Failed to label PR ${PR_NUMBER}</h4>"
echo "<pre>"
cat action.out
echo "</pre>"
} >> "$GITHUB_STEP_SUMMARY"
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
run: ./dev_tools/ci/size-labeler.sh
33 changes: 27 additions & 6 deletions dev_tools/ci/size-labeler.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,12 @@ function jq_stdin() {
local infile
infile="$(mktemp)"
readonly infile
local jq_status=0

cat >"${infile}"
jq_file "$@" "${infile}"
jq_file "$@" "${infile}" || jq_status="${?}"
rm "${infile}"
return "${jq_status}"
}

function jq_file() {
Expand All @@ -81,22 +84,38 @@ function jq_file() {
function api_call() {
local -r endpoint="${1// /%20}" # love that our label names have spaces...
local -r uri="https://api.github.com/repos/${GITHUB_REPOSITORY}"
local response
local curl_status=0
info "Calling: ${uri}/${endpoint}"
curl -sSL \
response="$(curl -sSL \
--fail-with-body \
--connect-timeout 10 --max-time 20 \
-H "Authorization: token ${GITHUB_TOKEN}" \
-H "Accept: application/vnd.github.v3.json" \
-H "X-GitHub-Api-Version:2022-11-28" \
-H "Content-Type: application/json" \
"${@:2}" \
"${uri}/${endpoint}"
)" || curl_status="${?}"
if [[ -n "${response}" ]]; then
cat <<<"${response}"
fi
if (( curl_status )); then
error "GitHub API call failed (curl exit $curl_status) for ${uri}/${endpoint}"
error "Response body:"
cat >&2 <<<"${response}"
fi
return "${curl_status}"
}

function compute_changes() {
local -r pr="$1"

local response
local change_info
local -r keys_filter='with_entries(select([.key] | inside(["changes", "filename"])))'
change_info="$(jq_stdin "map(${keys_filter})" <<<"$(api_call "pulls/${pr}/files")")"
response="$(api_call "pulls/${pr}/files")"
change_info="$(jq_stdin "map(${keys_filter})" <<<"${response}")"

local files total_changes
readarray -t files < <(jq_stdin -c '.[]' <<<"${change_info}")
Expand Down Expand Up @@ -131,8 +150,10 @@ function get_size_label() {
function prune_stale_labels() {
local -r pr="$1"
local -r size_label="$2"
local response
local existing_labels
existing_labels="$(jq_stdin -r '.labels[] | .name' <<<"$(api_call "pulls/${pr}")")"
response="$(api_call "pulls/${pr}")"
existing_labels="$(jq_stdin -r '.labels[] | .name' <<<"${response}")"
readarray -t existing_labels <<<"${existing_labels}"

local correctly_labeled=false
Expand All @@ -147,7 +168,7 @@ function prune_stale_labels() {
# If there is another size label, we need to remove it
if [[ -v "LIMITS[${label}]" ]]; then
info "Label '${label}' is stale, removing it."
api_call "issues/${pr}/labels/${label}" -X DELETE &>/dev/null
api_call "issues/${pr}/labels/${label}" -X DELETE >/dev/null
continue
fi
info "Label '${label}' is unknown, leaving it."
Expand Down Expand Up @@ -194,7 +215,7 @@ function main() {
correctly_labeled="$(prune_stale_labels "$PR_NUMBER" "${size_label}")"

if [[ "${correctly_labeled}" != true ]]; then
api_call "issues/$PR_NUMBER/labels" -X POST -d "{\"labels\":[\"${size_label}\"]}" &>/dev/null
api_call "issues/$PR_NUMBER/labels" -X POST -d "{\"labels\":[\"${size_label}\"]}" >/dev/null
info "Added label '${size_label}'"
fi
}
Expand Down