Skip to content
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

Fetch changed files from PR in chunks #3117

Merged
merged 2 commits into from
Nov 16, 2024

Conversation

bartgol
Copy link
Contributor

@bartgol bartgol commented Nov 15, 2024

There are page size limits when using rest API. By default, it returns max 30 items. One can force up to 100, but if there are more, we need to do some paging.

E.g.:

response=$(curl -s -H "Authorization: token ***" "https://api.github.com/repos/E3SM-Project/scream/pulls/3115/fies")
$ changed_files=$(echo "$response" | grep -o '"filename": *"[^"]*"' | sed 's/"filename": *//; s/"//g')
$ echo $changed_files | sed 's/ /\n/g' | wc -l
30
$ response=$(curl -s -H "Authorization: token ***" "https://api.github.com/repos/E3SM-Project/scream/pulls/3115/files?per_page=10000")
$ changed_files=$(echo "$response" | grep -o '"filename": *"[^"]*"' | sed 's/"filename": *//; s/"//g')
$ echo $changed_files | sed 's/ /\n/g' | wc -l
100
$ changed_files=""
$ page=1
$ while true; do
     response=$(curl -s -H "Authorization: token ***" "https://api.github.com/repos/E3SM-Project/scream/pulls/3115/files?per_page=100&page=$page");
     if [ -z "$response" ]; then
         break;
     fi;
     files=$(echo "$response" | grep -o '"filename": *"[^"]*"' | sed 's/"filename": *//; s/"//g');
     changed_files+="$files"$'\n';
     if [[ $(echo "$response" | jq '. | length') -lt 100 ]]; then
         break;
     fi;
     page=$((page + 1)); done
$ echo $changed_files | sed 's/ /\n/g' | wc -l
221

The authors would like to thank sandia AI chat bot for its invaluable help.

There are page size limits when using rest API
Copy link
Contributor

mergify bot commented Nov 15, 2024

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟠 Enforce checks passing

Waiting checks: gcc-cuda / ${{ matrix.build_type }}, gcc-cuda / dbg, gcc-cuda / opt, gcc-cuda / sp.

Make sure that checks are not failing on the PR, and reviewers approved

  • any of:
    • check-skipped={% raw %}gcc-cuda / ${{ matrix.build_type }}{% endraw %}
    • all of:
      • check-success="gcc-cuda / dbg"
      • check-success="gcc-cuda / opt"
      • check-success="gcc-cuda / sp"
  • #approved-reviews-by >= 1
  • #changes-requested-reviews-by == 0
  • any of:
    • all of:
      • check-success="gcc-openmp / dbg"
      • check-success="gcc-openmp / fpe"
      • check-success="gcc-openmp / opt"
      • check-success="gcc-openmp / sp"
    • check-skipped={% raw %}gcc-openmp / ${{ matrix.build_type }}{% endraw %}
  • any of:
    • all of:
      • check-success="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
      • check-success="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
      • check-success="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
      • check-success="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
    • check-skipped={% raw %}cpu-gcc / ${{ matrix.test.short_name }}{% endraw %}
  • any of:
    • check-success=cpu-gcc
    • check-skipped=cpu-gcc

@bartgol bartgol added the CI: workflow change approved Allow testing of PRs that alter a worfklow file label Nov 15, 2024
Copy link
Contributor

@mahf708 mahf708 left a comment

Choose a reason for hiding this comment

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

I think these actions/workflows are too complicated and error prone, but we will audit them in more detail later.

@bartgol bartgol merged commit ed62e69 into master Nov 16, 2024
14 of 18 checks passed
@bartgol bartgol deleted the bartgol/workflows/fix-pre-process-pr branch November 16, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix CI: workflow change approved Allow testing of PRs that alter a worfklow file workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants