Add Github Run ID logic locally to pr scripts - #165
Conversation
| # Generate the artifact name | ||
| artifact_name="$(RAPIDS_NO_PKG_EXTENSION=true rapids-package-name "conda_${package_type}")" | ||
| pkg_name="$(RAPIDS_NO_PKG_EXTENSION=true rapids-package-name "conda_${package_type}")" | ||
| github_run_id=$(gh run list --repo "${RAPIDS_REPOSITORY}" --branch "${RAPIDS_REF_NAME}" --commit "${RAPIDS_SHA}" --json databaseId --jq '.[0] | .databaseId') |
There was a problem hiding this comment.
Does this need to use rapids-retry in case the GitHub API fails? Will the $(...) parse the output correctly if it is retried?
| github_run_id=$(gh run list --repo "${RAPIDS_REPOSITORY}" --branch "${RAPIDS_REF_NAME}" --commit "${RAPIDS_SHA}" --json databaseId --jq '.[0] | .databaseId') | |
| github_run_id=$(rapids-retry gh run list --repo "${RAPIDS_REPOSITORY}" --branch "${RAPIDS_REF_NAME}" --commit "${RAPIDS_SHA}" --json databaseId --jq '.[0] | .databaseId') |
There was a problem hiding this comment.
This PR (#164) changes the way rapids-retry works with stdout. The --quiet flag being added will ensure that that none of the rapids-logger messages are captured by this variable.
Once those changes have been approved and merged, I will make those changes here.
There was a problem hiding this comment.
Can this logic be refactored into a common file? See https://github.com/rapidsai/gha-tools/blob/main/tools/_rapids-download-from-s3 for an example.
There was a problem hiding this comment.
Yes, that's a great idea.
Refactored the logic into a common file called _rapids-get-pr-artifact-github, mirroring the PR artifact flow used earlier.
| # If commit is not provided, get the latest commit on the PR | ||
| if [[ -z "${commit}" ]]; then | ||
| commit=$(rapids-retry gh pr view "${pr}" --repo rapidsai/"${repo}" --json headRefOid --jq '.headRefOid') | ||
| commit=$(git ls-remote https://github.com/rapidsai/"${repo}".git refs/heads/pull-request/"${pr}" | cut -f1) |
There was a problem hiding this comment.
These two seem the same -- is there a reason to change this and miss out on the retries?
There was a problem hiding this comment.
Refer to the comment above about the rapids-retry PR. (#164).
However, using gh lets us access PR artifacts from private repositories, so I will switch it back to using gh pr view
| github_run_id=$(gh run list --repo "${RAPIDS_REPOSITORY}" --branch "${RAPIDS_REF_NAME}" --commit "${RAPIDS_SHA}" --json databaseId --jq '.[0] | .databaseId') | ||
| unzip_dest="${RAPIDS_UNZIP_DIR:-$(mktemp -d)}" | ||
|
|
||
| unzip_dir=$(rapids-download-from-github "${artifact_name}") | ||
| echo -n "${unzip_dir}" | ||
| rapids-echo-stderr "Downloading and decompressing ${pkg_name} from Run ID ${github_run_id} into ${unzip_dest}" | ||
| rapids-retry gh run download "${github_run_id}" --repo "${RAPIDS_REPOSITORY}" --name "${pkg_name}" --dir "${unzip_dest}" |
There was a problem hiding this comment.
so it seems like this is a workaround for the current behavior of RAPIDS_BUILD_TYPE=pull-request in rapids-github-run-id -- should we change that instead?
There was a problem hiding this comment.
In rapids-github-run-id, the default value of the Run ID is the GITHUB_RUN_ID environment variable set by Github. Making this change there wouldn't help in the usecase where PR artifacts are needed as part of a Github Workflow run.
Making this change here would be the best option for both Actions and for reproducing CI locally.
|
This seems fine to me - @gforsyth please take a final pass when you can, and let's merge. I'd like to test some CI artifacts downstream as soon as this is fixed. |
3 participants
This PR addresses an unwanted implication of using the
GITHUB_RUN_IDenv-var in determining the Github Actions run ID to download build artifacts. (see #162)This env-var is set while using Github Actions, so the scripts take up the ID of the current Github run, instead of determining the ID of the original run from which PR artifacts are to be downloaded.
This PR localizes all the logic necessary to determine the Run ID of the actions run triggered on the specific commit of the PR, and ensures that all these operations are done in a subshell to prevent any unwanted env-var changes elsewhere.