Use pagination when requesting github API #1168

mboukhalfa · 2023-03-06T17:17:42Z

Fix #1162

mboukhalfa · 2023-03-06T17:29:19Z

/test-ubuntu-integration-e2e-main

mboukhalfa · 2023-03-06T17:29:34Z

/test-centos-integration-release-1-3

tuminoid

Some changes needed.

tuminoid · 2023-03-07T08:05:25Z

lib/releases.sh

  if [ -z "${GITHUB_TOKEN:-}" ]; then
-    release="$(curl -sL "${1}")" || ( set -x && exit 1 )
+    last_page=$(curl -s -I "${1}" -H "Authorization: token $GITHUB_TOKEN" | grep '^link:' | sed -e 's/^link:.*page=//g' -e 's/>.*$//g') || (set -x && exit 1)


(set -x && exit 1) creates a subshell, the parenthesis does that, so the exit 1 exits only that subshell, and not the script. If you want to perform two actions, use curly braces, {set -x && exit 1}.

Also, you want to assign the subshell output of $( ... ) with quotes, so do last_page="$( ... )"

shellcheck complaining about {set -x && exit 1} and ${set -x && exit 1} not working as expected do you know what is wrong

${set -x ...} is definitely wrong, but what does it complain about { set -x ... } ?

You need semicolon at the end { set -x && exit 1; }.

thanks! done

tuminoid · 2023-03-07T08:05:37Z

lib/releases.sh

  else
-    release="$(curl -H "Authorization: token ${GITHUB_TOKEN}" -sL "${1}")" || ( set -x && exit 1 )
+    last_page=$(curl -s -I "${1}" | grep '^link:' | sed -e 's/^link:.*page=//g' -e 's/>.*$//g') || (set -x && exit 1)


tuminoid · 2023-03-07T08:05:55Z

lib/releases.sh

-  # selection is possible given specific enough prefix, like v1.3.0-pre
-  release_tag="$(echo "$release" | jq -r "[.[].tag_name | select( startswith(\"${2:-}\"))] | .[0]")"
+  # default last page to 1
+  last_page=${last_page:-1}


Quotes do no harm here either.

lib/releases.sh

tuminoid · 2023-03-07T08:08:23Z

lib/releases.sh

+    url="${1}?page=${current_page}"
+
+    if [ -z "${GITHUB_TOKEN:-}" ]; then
+      release="$(curl -sL "${url}")" || (set -x && exit 1)


same as the first one

tuminoid · 2023-03-07T08:08:28Z

lib/releases.sh

+    if [ -z "${GITHUB_TOKEN:-}" ]; then
+      release="$(curl -sL "${url}")" || (set -x && exit 1)
+    else
+      release="$(curl -H "Authorization: token ${GITHUB_TOKEN}" -sL "${url}")" || (set -x && exit 1)


same as the first one

tuminoid · 2023-03-07T08:08:46Z

lib/releases.sh

+    # The order is exactly as released in Github.
+    # Downside is that selecting official releases only isn't possible, while pre-release
+    # selection is possible given specific enough prefix, like v1.3.0-pre
+    release_tag="$(echo "$release" | jq -r "[.[].tag_name | select( startswith(\"${2:-}\"))] | .[0]")"


Add curlys to $release

tuminoid · 2023-03-07T08:08:56Z

lib/releases.sh

+    # selection is possible given specific enough prefix, like v1.3.0-pre
+    release_tag="$(echo "$release" | jq -r "[.[].tag_name | select( startswith(\"${2:-}\"))] | .[0]")"
+    # if release tag found
+    if [[ "$release_tag" != "null" ]]; then


mboukhalfa · 2023-03-07T09:31:33Z

/test-centos-integration-release-1-3
/test-ubuntu-integration-e2e-main

mboukhalfa · 2023-03-07T09:59:52Z

/test-ubuntu-integration-e2e-main

mboukhalfa · 2023-03-07T10:58:51Z

/test-ubuntu-integration-e2e-main

Rozzii · 2023-03-07T11:54:02Z

/lgtm

mboukhalfa · 2023-03-07T11:54:42Z

/test-centos-integration-release-1-3

furkatgofurov7 · 2023-03-07T14:32:56Z

/test-ubuntu-e2e-integration-main

mboukhalfa · 2023-03-08T07:27:52Z

/assign @furkatgofurov7
cc @lentzi90

mboukhalfa · 2023-03-08T07:29:31Z

/hold

mboukhalfa · 2023-03-08T07:38:45Z

/test-centos-integration-release-1-3
/test-ubuntu-integration-e2e-main
/hold cancel

lentzi90 · 2023-03-08T08:38:19Z

lib/releases.sh

+  command -v jq &>/dev/null && echo "Failed to fetch CAPI release from Github" && exit 1
 fi

 if [[ "$CAPM3RELEASE" == "" ]]; then
-  command -v jq &> /dev/null && echo "Failed to fetch CAPM3 release from Github" && exit 1
+  command -v jq &>/dev/null && echo "Failed to fetch CAPM3 release from Github" && exit 1
 fi

 if [[ "$BMORELEASE" == "" ]]; then
-  command -v jq &> /dev/null && echo "Failed to fetch BMO release from Github" && exit 1
+  command -v jq &>/dev/null && echo "Failed to fetch BMO release from Github" && exit 1
 fi


Why remove the space here? I think it is easier to read with the space so we should add it back.

it was auto reformatted by editor formatter! do we have coding style for that

@tuminoid any input on this? I'm inclined to approve it as just a matter of preference if there are no objections.

That space is completely up to personal taste. I tend to have the space there for readability, but it doesn't really matter.

Alright I'm putting lgtm then 🙂

furkatgofurov7

Checked locally, it works fine

/approve
/hold to address the comments from @lentzi90

metal3-io-bot · 2023-03-08T08:53:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: furkatgofurov7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [furkatgofurov7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mboukhalfa · 2023-03-08T11:53:50Z

/test-ubuntu-e2e-integration-main

mboukhalfa · 2023-03-08T13:32:34Z

/test-ubuntu-e2e-integration-main

lentzi90 · 2023-03-08T13:43:27Z

/lgtm

mboukhalfa · 2023-03-08T13:59:33Z

/hold cancel

metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 6, 2023

mboukhalfa force-pushed the Use-github-pagination/mohammed branch from 7bb87a3 to d14728d Compare March 6, 2023 17:20

tuminoid self-requested a review March 6, 2023 18:35

tuminoid requested changes Mar 7, 2023

View reviewed changes

mboukhalfa force-pushed the Use-github-pagination/mohammed branch from d14728d to be8b0df Compare March 7, 2023 08:25

mboukhalfa requested a review from tuminoid March 7, 2023 08:27

mboukhalfa force-pushed the Use-github-pagination/mohammed branch 2 times, most recently from 4ec57f1 to ba20246 Compare March 7, 2023 09:30

mboukhalfa force-pushed the Use-github-pagination/mohammed branch from ba20246 to cfe8652 Compare March 7, 2023 10:58

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2023

metal3-io-bot assigned furkatgofurov7 Mar 8, 2023

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2023

Use pagination when requesting github API

8376ca5

mboukhalfa force-pushed the Use-github-pagination/mohammed branch from cfe8652 to 8376ca5 Compare March 8, 2023 07:33

metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 8, 2023

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2023

lentzi90 reviewed Mar 8, 2023

View reviewed changes

furkatgofurov7 approved these changes Mar 8, 2023

View reviewed changes

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2023

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 8, 2023

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 8, 2023

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2023

metal3-io-bot merged commit 78ca7ab into metal3-io:main Mar 8, 2023

mboukhalfa deleted the Use-github-pagination/mohammed branch March 8, 2023 14:07

mboukhalfa mentioned this pull request Apr 18, 2023

Reduce api queries when getting latest release tag #1219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pagination when requesting github API #1168

Use pagination when requesting github API #1168

mboukhalfa commented Mar 6, 2023

mboukhalfa commented Mar 6, 2023

mboukhalfa commented Mar 6, 2023

tuminoid left a comment

tuminoid Mar 7, 2023

mboukhalfa Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

mboukhalfa Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

tuminoid Mar 7, 2023

mboukhalfa commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

Rozzii commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

furkatgofurov7 commented Mar 7, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

lentzi90 Mar 8, 2023

mboukhalfa Mar 8, 2023 •

edited

Loading

lentzi90 Mar 8, 2023 •

edited

Loading

tuminoid Mar 8, 2023

lentzi90 Mar 8, 2023

furkatgofurov7 left a comment

metal3-io-bot commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

lentzi90 commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

Use pagination when requesting github API #1168

Use pagination when requesting github API #1168

Conversation

mboukhalfa commented Mar 6, 2023

mboukhalfa commented Mar 6, 2023

mboukhalfa commented Mar 6, 2023

tuminoid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mboukhalfa commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

Rozzii commented Mar 7, 2023

mboukhalfa commented Mar 7, 2023

furkatgofurov7 commented Mar 7, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

Choose a reason for hiding this comment

mboukhalfa Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

lentzi90 Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

furkatgofurov7 left a comment

Choose a reason for hiding this comment

metal3-io-bot commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

lentzi90 commented Mar 8, 2023

mboukhalfa commented Mar 8, 2023

mboukhalfa Mar 8, 2023 •

edited

Loading

lentzi90 Mar 8, 2023 •

edited

Loading