You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/16 09:41:36 UTC
[GitHub] [pulsar] lhotari opened a new pull request, #17693: [feat][CI] Add ready-to-test solution to reduce GitHub Actions resource consumption
lhotari opened a new pull request, #17693:
URL: https://github.com/apache/pulsar/pull/17693
### Motivation
To reduce resource consumption, PRs would require an initial review before all tests would be run. PR authors would be required to run tests in their own forks to make sure that tests pass and before the initial review is approved.
This way PR authors would have CI feedback without causing resource shortage to apache/pulsar CI.
Please check the proposal email on dev mailing list: https://lists.apache.org/thread/gwfmxmxlhtsjn17sxxc367jcs4pcwofz
### Modifications
- stop non-doc CI builds if the PR doesn't have the "ready-to-test" label.
- Provide detailed instructions for the PR author:
example from testing this:
(https://github.com/lhotari/pulsar/actions/runs/3066810135)
(https://github.com/lhotari/pulsar/actions/runs/3066810135/jobs/4952402230)
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [x] `doc-required`
(Your PR needs to update docs and you will update later)
Contributor guide needs updates.
- [ ] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973018381
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
Review Comment:
`exit 1`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973021504
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
+ echo "Review decision for PR #${PR_NUM} in repository ${REPO_OWNER}/${REPO_NAME} is ${PR_REVIEW_DECISION}"
+ if [[ "$PR_REVIEW_DECISION" != "APPROVED" ]]; then
+ FORK_REPO_URL=$(jq -r '.pull_request.head.repo.html_url' "$GITHUB_EVENT_PATH")
+ PR_BRANCH_LABEL=$(jq -r '.pull_request.head.label' "$GITHUB_EVENT_PATH")
+ PR_URL=$(jq -r '.pull_request.html_url' "$GITHUB_EVENT_PATH")
+ >&2 tee -a "$GITHUB_STEP_SUMMARY" <<EOF
+
+# Instructions for proceeding with the pull request:
+
+apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
+GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
+pull requests that are executed in a forked repository.
+
+1. Go to ${FORK_REPO_URL} and ensure that your branch is up to date with https://github.com/apache/pulsar
+ Sync your fork if it's behind.
+2. Open a pull request to your own fork. You can use this link to create the pull request in
+ your own fork:
+ ${FORK_REPO_URL}/compare/master...${PR_BRANCH_LABEL}?expand=1
+3. Edit the description of the pull request ${PR_URL} and add the link to the PR that you opened to your own fork
+ so that the reviewer can verify that tests pass in your own fork.
+4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
+ and any changes made during the review. You as a PR author are responsible for following up on test failures.
+ Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
+ after checking that the flaky test isn't already reported.
+5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
+ repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
+ An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
+ by Apache Pulsar committers.
+6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
+
+If you have any trouble, please join the #contributor channel on Pulsar Slack to get real-time support.
+Instructions for joining Pulsar Slack: https://pulsar.apache.org/community#section-discussions
Review Comment:
> mentioning GitHub discussions as well could extend the reach to everyone.
Yes. My opinion here is that Slack is not quite convenient also, or at least for me XD. In all issue templates we mention dev@ and Discussions and they don't need to add a link jump to https://pulsar.apache.org/community#section-discussions.
I'd prefer to show only one approach to avoid "decidophobia".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249528604
/pulsarbot rerun-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249565192
Latest example of the markdown (this is a result of some local testing). Please notice the "Create PR in fork for running tests" link for creating the PR.
# Instructions for proceeding with the pull request:
apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
pull requests that are executed in a forked repository.
1. Go to https://github.com/lhotari/pulsar and ensure that your branch is up to date with https://github.com/apache/pulsar
Sync your fork if it's behind.
2. Open a pull request to your own fork. You can use this link to create the pull request in
your own fork:
[Create PR in fork for running tests](https://github.com/lhotari/pulsar/compare/master...lhotari:lh-pulsarci-ready-to-test?expand=1&title=%5Brun-tests%5D%20lh%20pulsarci%20ready%20to%20test&body=This%20PR%20is%20for%20running%20tests%20for%20upstream%20PR%20https%3A%2F%2Fgithub.com%2Flhotari%2Fpulsar%2Fpull%2F83.%0A%0A%3C!--%20Before%20creating%20this%20PR%2C%20please%20ensure%20that%20the%20fork%20https%3A%2F%2Fgithub.com%2Flhotari%2Fpulsar%20is%20up%20to%20date%20with%20https%3A%2F%2Fgithub.com%2Fapache%2Fpulsar%20--%3E)
3. Edit the description of the pull request https://github.com/lhotari/pulsar/pull/83 and add the link to the PR that you opened to your own fork
so that the reviewer can verify that tests pass in your own fork.
4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
and any changes made during the review. You as a PR author are responsible for following up on test failures.
Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
after checking that the flaky test isn't already reported.
5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
by Apache Pulsar committers.
6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
If you have any trouble you can get support in multiple ways:
* by sending email to the [dev mailing list](mailto:dev@pulsar.apache.org) ([subscribe](mailto:dev-subscribe@pulsar.apache.org))
* on the [#contributors channel on Pulsar Slack](https://apache-pulsar.slack.com/channels/contributors) ([join](https://pulsar.apache.org/community#section-discussions))
* in apache/pulsar [GitHub discussions Q&A](https://github.com/apache/pulsar/discussions/categories/q-a)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973013377
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -60,6 +60,11 @@ jobs:
echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"
echo "::set-output name=cpp_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.cpp_count) && fromJSON(steps.changes.outputs.cpp_count) > 0 }}"
+ - name: Check if the PR has been approved for testing
Review Comment:
Correspondingly update job name? Perhaps "Preconditions". Ditto other workflows. These are beyond "Changed files check".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249556637
In the most recent commit 2aa2a80c1bb593e3acd3e945f012344dbbf30c37 to this PR, there will be an easy way to create the PR in your own fork by just clicking the link. By default the PR title will be prefixed with "[run-tests] " and the body will be "This PR is for running tests for upstream PR ${PR_URL}."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
momo-jun commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1255892364
CC @Anonymitaet for the contribution guide updates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973035725
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
Review Comment:
I tested that it works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari merged pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari merged PR #17693:
URL: https://github.com/apache/pulsar/pull/17693
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249562991
> The `ready-to-test` label has been added, it works fine.
@nodece It also accepts the review approval as a way to allow tests to run. I added that based on feedback from @merlimat and @codelipenghui .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973010235
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
+ echo "Review decision for PR #${PR_NUM} in repository ${REPO_OWNER}/${REPO_NAME} is ${PR_REVIEW_DECISION}"
+ if [[ "$PR_REVIEW_DECISION" != "APPROVED" ]]; then
+ FORK_REPO_URL=$(jq -r '.pull_request.head.repo.html_url' "$GITHUB_EVENT_PATH")
+ PR_BRANCH_LABEL=$(jq -r '.pull_request.head.label' "$GITHUB_EVENT_PATH")
+ PR_URL=$(jq -r '.pull_request.html_url' "$GITHUB_EVENT_PATH")
+ >&2 tee -a "$GITHUB_STEP_SUMMARY" <<EOF
+
+# Instructions for proceeding with the pull request:
+
+apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
+GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
+pull requests that are executed in a forked repository.
+
+1. Go to ${FORK_REPO_URL} and ensure that your branch is up to date with https://github.com/apache/pulsar
+ Sync your fork if it's behind.
+2. Open a pull request to your own fork. You can use this link to create the pull request in
+ your own fork:
+ ${FORK_REPO_URL}/compare/master...${PR_BRANCH_LABEL}?expand=1
+3. Edit the description of the pull request ${PR_URL} and add the link to the PR that you opened to your own fork
+ so that the reviewer can verify that tests pass in your own fork.
+4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
+ and any changes made during the review. You as a PR author are responsible for following up on test failures.
+ Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
+ after checking that the flaky test isn't already reported.
+5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
+ repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
+ An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
+ by Apache Pulsar committers.
+6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
+
+If you have any trouble, please join the #contributor channel on Pulsar Slack to get real-time support.
+Instructions for joining Pulsar Slack: https://pulsar.apache.org/community#section-discussions
Review Comment:
I suggest we guide contributors to the dev@ mailing list：
```suggestion
If you have any trouble, please send an email to the [dev mailing list](mailto:dev@pulsar.apache.org) ([subscribe](mailto:dev-subscribe@pulsar.apache.org)) to get support.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973037970
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
Yes. "man yq" says "Keep in mind JSON is a subset of YAML." .
There's not really a need to use yq here, it could be replaced with jq to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973048689
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
+ echo "Review decision for PR #${PR_NUM} in repository ${REPO_OWNER}/${REPO_NAME} is ${PR_REVIEW_DECISION}"
+ if [[ "$PR_REVIEW_DECISION" != "APPROVED" ]]; then
+ FORK_REPO_URL=$(jq -r '.pull_request.head.repo.html_url' "$GITHUB_EVENT_PATH")
+ PR_BRANCH_LABEL=$(jq -r '.pull_request.head.label' "$GITHUB_EVENT_PATH")
+ PR_URL=$(jq -r '.pull_request.html_url' "$GITHUB_EVENT_PATH")
+ >&2 tee -a "$GITHUB_STEP_SUMMARY" <<EOF
+
+# Instructions for proceeding with the pull request:
+
+apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
+GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
+pull requests that are executed in a forked repository.
+
+1. Go to ${FORK_REPO_URL} and ensure that your branch is up to date with https://github.com/apache/pulsar
+ Sync your fork if it's behind.
+2. Open a pull request to your own fork. You can use this link to create the pull request in
+ your own fork:
+ ${FORK_REPO_URL}/compare/master...${PR_BRANCH_LABEL}?expand=1
+3. Edit the description of the pull request ${PR_URL} and add the link to the PR that you opened to your own fork
+ so that the reviewer can verify that tests pass in your own fork.
+4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
+ and any changes made during the review. You as a PR author are responsible for following up on test failures.
+ Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
+ after checking that the flaky test isn't already reported.
+5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
+ repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
+ An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
+ by Apache Pulsar committers.
+6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
+
+If you have any trouble, please join the #contributor channel on Pulsar Slack to get real-time support.
+Instructions for joining Pulsar Slack: https://pulsar.apache.org/community#section-discussions
Review Comment:
addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249560883
Also added a notice `<!-- Before creating this PR, please ensure that the fork $FORK_REPO_URL is up to date with https://github.com/apache/pulsar -->` to the generated PR body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973030213
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
Review Comment:
```suggestion
PR_REVIEW_DECISION=$(curl -s -H "Authorization: Bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
```
Do you test this line? I don't know whether it's case sensitive but [the document writes `Bearer`](https://docs.github.com/en/rest/overview/other-authentication-methods#authenticating-for-saml-sso).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973014639
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
+ echo "Review decision for PR #${PR_NUM} in repository ${REPO_OWNER}/${REPO_NAME} is ${PR_REVIEW_DECISION}"
+ if [[ "$PR_REVIEW_DECISION" != "APPROVED" ]]; then
+ FORK_REPO_URL=$(jq -r '.pull_request.head.repo.html_url' "$GITHUB_EVENT_PATH")
+ PR_BRANCH_LABEL=$(jq -r '.pull_request.head.label' "$GITHUB_EVENT_PATH")
+ PR_URL=$(jq -r '.pull_request.html_url' "$GITHUB_EVENT_PATH")
+ >&2 tee -a "$GITHUB_STEP_SUMMARY" <<EOF
+
+# Instructions for proceeding with the pull request:
+
+apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
+GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
+pull requests that are executed in a forked repository.
+
+1. Go to ${FORK_REPO_URL} and ensure that your branch is up to date with https://github.com/apache/pulsar
+ Sync your fork if it's behind.
+2. Open a pull request to your own fork. You can use this link to create the pull request in
+ your own fork:
+ ${FORK_REPO_URL}/compare/master...${PR_BRANCH_LABEL}?expand=1
+3. Edit the description of the pull request ${PR_URL} and add the link to the PR that you opened to your own fork
+ so that the reviewer can verify that tests pass in your own fork.
+4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
+ and any changes made during the review. You as a PR author are responsible for following up on test failures.
+ Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
+ after checking that the flaky test isn't already reported.
+5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
+ repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
+ An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
+ by Apache Pulsar committers.
+6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
+
+If you have any trouble, please join the #contributor channel on Pulsar Slack to get real-time support.
+Instructions for joining Pulsar Slack: https://pulsar.apache.org/community#section-discussions
Review Comment:
Mentioning dev mailing is worthwhile. However, I wouldn't remove the mention about Slack since there's a high barrier to join the dev mailing list or post to the mailing list. I think mentioning GitHub discussions as well could extend the reach to everyone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1250241072
@lhotari Perhaps we can also always should these instructions on pull requests created. All new pull requests should encounter this case.
The effect looks like https://github.com/pingcap/tidb/pull/37911#issuecomment-1250238271 and we can implement it with GitHub Actions (It's about up to 10~30 new lightweight workflow runs ([metric](https://play.clickhouse.com/play?user=play#U0VMRUNUIHRvU3RhcnRPZkRheShjcmVhdGVkX2F0KSBkLCBDT1VOVCgqKSB0CkZST00gZ2l0aHViX2V2ZW50cwpXSEVSRSByZXBvX25hbWUgPSAnYXBhY2hlL3B1bHNhcicKICBBTkQgZXZlbnRfdHlwZSA9ICdQdWxsUmVxdWVzdEV2ZW50JwogIEFORCBhY3Rpb24gPSAnb3BlbmVkJwpHUk9VUCBCWSBkCk9SREVSIEJZIGQgREVTQwpMSU1JVCAxMA==))).
The reason is that the summary in check result doesn't show in the main thread of the pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973053895
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,61 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! jq -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
```suggestion
# check ready-to-test label
if jq -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
return 0
fi
```
I'd say the first time I don't notice we check the label and return on matched here. So I'd recall this suggestion. But it should not be a blocker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973014838
##########
.github/PULL_REQUEST_TEMPLATE.md:
##########
@@ -76,3 +76,25 @@ This change added tests and can be verified as follows:
- [ ] `doc-complete`
(Docs have been already added)
+
+### Matching PR in forked repository
+
+PR in forked repository: <!-- ENTER URL HERE
+
+After opening this PR, the build in apache/pulsar will fail and instructions will
+be provided for opening a PR in the PR author's forked repository.
+
+apache/pulsar pull requests should be first tested in your own fork since the
+apache/pulsar CI based on GitHub Actions has constrained resources and quota.
+GitHub Actions provides separate quota for pull requests that are executed in
+a forked repository.
+
+The tests will be run in the forked repository until all PR review comments have
+been handled, the tests pass and the PR is approved by a reviewer.
+
+-->
+
+
+
+
+
Review Comment:
```suggestion
```
Trim.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249308050
Here's the latest example of the instructions that get printed as an error message and in the workflow summary:
# Instructions for proceeding with the pull request:
apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on
GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for
pull requests that are executed in a forked repository.
1. Go to https://github.com/lhotari/pulsar and ensure that your branch is up to date with https://github.com/apache/pulsar
Sync your fork if it's behind.
2. Open a pull request to your own fork. You can use this link to create the pull request in
your own fork:
https://github.com/lhotari/pulsar/compare/master...lhotari:lh-pulsarci-ready-to-test?expand=1
3. Edit the description of the pull request https://github.com/lhotari/pulsar/pull/83 and add the link to the PR that you opened to your own fork
so that the reviewer can verify that tests pass in your own fork.
4. Ensure that tests pass in your own fork. Your own fork will be used to run the tests during the PR review
and any changes made during the review. You as a PR author are responsible for following up on test failures.
Please report any flaky tests as new issues at https://github.com/apache/pulsar/issues
after checking that the flaky test isn't already reported.
5. When the PR is approved, it will be possible to restart the Pulsar CI workflow within apache/pulsar
repository by adding a comment "/pulsarbot rerun-failure-checks" to the PR.
An alternative for the PR approval is to add a ready-to-test label to the PR. This can be done
by Apache Pulsar committers.
6. When tests pass on the apache/pulsar side, the PR can be merged by a Apache Pulsar Committer.
If you have any trouble, please join the #contributor channel on Pulsar Slack to get real-time support.
Instructions for joining Pulsar Slack: https://pulsar.apache.org/community#section-discussions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973074213
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,61 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! jq -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
Good point. I revisited the logic as you have suggested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973035498
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
+ # check if the PR has been approved
+ PR_NUM=$(jq -r '.pull_request.number' "${GITHUB_EVENT_PATH}")
+ REPO_FULL_NAME=$(jq -r '.repository.full_name' "${GITHUB_EVENT_PATH}")
+ REPO_NAME=$(basename "${REPO_FULL_NAME}")
+ REPO_OWNER=$(dirname "${REPO_FULL_NAME}")
+ # use graphql query to find out reviewDecision
+ PR_REVIEW_DECISION=$(curl -s -H "Authorization: bearer $GITHUB_TOKEN" -X POST -d '{"query": "query { repository(name: \"'${REPO_NAME}'\", owner: \"'${REPO_OWNER}'\") { pullRequest(number: '${PR_NUM}') { reviewDecision } } }"}' https://api.github.com/graphql |jq -r '.data.repository.pullRequest.reviewDecision')
Review Comment:
I copy-pasted the example from GitHub docs: https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#communicating-with-graphql . It's lower case there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973031785
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
Nit: return on ready-to-test here to improve code comprehension.
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
Nit: return on `ready-to-test` matched here to improve code comprehension.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973039903
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
replaced with jq
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973015508
##########
.github/workflows/ci-cpp-build.yaml:
##########
@@ -54,6 +53,12 @@ jobs:
id: check_changes
run: |
echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"
+ echo "::set-output name=cpp_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.cpp_count) && fromJSON(steps.changes.outputs.cpp_count) > 0 }}"
Review Comment:
sure, it was missing from the go workflow so it was added just to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973014087
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -60,6 +60,11 @@ jobs:
echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"
echo "::set-output name=cpp_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.cpp_count) && fromJSON(steps.changes.outputs.cpp_count) > 0 }}"
+ - name: Check if the PR has been approved for testing
Review Comment:
Yes, makes sense to rename to "Preconditions"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973014405
##########
.github/workflows/ci-cpp-build.yaml:
##########
@@ -54,6 +53,12 @@ jobs:
id: check_changes
run: |
echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"
+ echo "::set-output name=cpp_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.cpp_count) && fromJSON(steps.changes.outputs.cpp_count) > 0 }}"
Review Comment:
```suggestion
```
This is exact the CPP workflow. We can remove the `cpp_only` output instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973050715
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -60,6 +60,11 @@ jobs:
echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"
echo "::set-output name=cpp_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.cpp_count) && fromJSON(steps.changes.outputs.cpp_count) > 0 }}"
+ - name: Check if the PR has been approved for testing
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973024131
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
+ fi
+ # check ready-to-test label
+ if ! yq eval -e '.pull_request.labels[] | .name | select(. == "ready-to-test")' "$GITHUB_EVENT_PATH" &> /dev/null; then
Review Comment:
Out of curiosity, I suppose `GITHUB_EVENT_PATH` represents a JSON file, so `yq` can process JSON files also?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#issuecomment-1249561652
The `ready-to-test` label has been added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17693: [feat][CI] Add approval solution to reduce GitHub Actions resource consumption
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17693:
URL: https://github.com/apache/pulsar/pull/17693#discussion_r973042594
##########
build/pulsar_ci_tool.sh:
##########
@@ -154,6 +154,59 @@ function ci_move_test_reports() {
)
}
+function ci_check_ready_to_test() {
+ if [[ -z "$GITHUB_EVENT_PATH" ]]; then
+ >&2 echo "GITHUB_EVENT_PATH isn't set"
+ return 1
Review Comment:
return works here too. I replaced the other occurence of `exit 1` with `return 1` to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org