You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/04/20 12:28:45 UTC
[GitHub] [spark] EnricoMi opened a new pull request, #36285: Check for changes only if changes are being built / tested (not on forks)
EnricoMi opened a new pull request, #36285:
URL: https://github.com/apache/spark/pull/36285
### What changes were proposed in this pull request?
The `precondition` job in the `build-and-test` workflow should be skipped on fork repositories as all subsequent / dependent jobs are already skipped.
### Why are the changes needed?
The `build and test` workflow is scheduled to run six times a day. The jobs then only run for `type == 'scheduled'` inside apache/spark repository, not for forks. However, the `precondition` job always runs, even though all subsequent jobs are skipped on forks.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a diff in pull request #36285: [SPARK-38970][CI] Check for changes only if changes are being built / tested (not on forks)
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r854708005
##########
.github/workflows/build_and_test.yml:
##########
@@ -106,6 +106,10 @@ jobs:
name: Check changes
runs-on: ubuntu-20.04
needs: configure-jobs
+ # Run scheduled jobs for Apache Spark only
+ # Run regular jobs for commit in both Apache Spark and forked repository
+ if: >-
+ (github.repository == 'apache/spark' && needs.configure-jobs.outputs.type == 'scheduled') || (needs.configure-jobs.outputs.type == 'regular')
Review Comment:
I guess `pyspark-coverage-scheduled` should also be here because `pyspark-coverage-scheduled` are also triggered by fork repo and run a redundant precondition?
https://github.com/apache/spark/blob/a2ac450896fd089bdbdf5447a336228fe9f8b879/.github/workflows/build_and_test.yml#L295
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #36285: [SPARK-38970][CI] Check for changes only if changes are being built / tested (not on forks)
Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1104635830
BTW, this is only skip precondtion (Check changes) but `configure-jobs` is still running in fork repo:
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1109569693
@Yikun @HyukjinKwon I have simplified logic as 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1110425405
Merged to master.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36285: [SPARK-38970][CI] Check for changes only if changes are being built / tested (not on forks)
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1106231169
You are right, moving the logic into `configure-jobs`, on which all other jobs depend on, allows to disable the entire workflow for `scheduled` events on forks and simplify logic in dependent jobs.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
URL: https://github.com/apache/spark/pull/36285
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858528703
##########
.github/workflows/build_and_test.yml:
##########
@@ -48,6 +48,11 @@ jobs:
configure-jobs:
name: Configure jobs
runs-on: ubuntu-20.04
+ # All other jobs in this workflow depend on this job,
+ # so the entire workflow is skipped when these conditions evaluate to false:
+ # Run all jobs for Apache Spark repository
+ # Run only non-scheduled jobs for forked repositories
Review Comment:
Good notes, but we'd better to still keep above notes (comments above) to mention fork/upstream trigger scopre
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1110085716
> Could also have a local test on your repo? And comments the link in here?
Good idea! I have changed the schedule for all six crons and tested them here: https://github.com/EnricoMi/spark/actions/workflows/build_and_test.yml?query=event%3Aschedule
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36285: [SPARK-38970][CI] Check for changes only if changes are being built / tested (not on forks)
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1104221263
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858528703
##########
.github/workflows/build_and_test.yml:
##########
@@ -48,6 +48,11 @@ jobs:
configure-jobs:
name: Configure jobs
runs-on: ubuntu-20.04
+ # All other jobs in this workflow depend on this job,
+ # so the entire workflow is skipped when these conditions evaluate to false:
+ # Run all jobs for Apache Spark repository
+ # Run only non-scheduled jobs for forked repositories
Review Comment:
Good notes, but we'd better to still keep above notes to mention fork/upstream trigger scopre
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858997393
##########
.github/workflows/build_and_test.yml:
##########
@@ -48,6 +48,11 @@ jobs:
configure-jobs:
name: Configure jobs
runs-on: ubuntu-20.04
+ # All other jobs in this workflow depend on this job,
+ # so the entire workflow is skipped when these conditions evaluate to false:
+ # Run all jobs for Apache Spark repository
+ # Run only non-scheduled jobs for forked repositories
Review Comment:
I have adjusted the comments and the one condition.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858528703
##########
.github/workflows/build_and_test.yml:
##########
@@ -48,6 +48,11 @@ jobs:
configure-jobs:
name: Configure jobs
runs-on: ubuntu-20.04
+ # All other jobs in this workflow depend on this job,
+ # so the entire workflow is skipped when these conditions evaluate to false:
+ # Run all jobs for Apache Spark repository
+ # Run only non-scheduled jobs for forked repositories
Review Comment:
Good notes, but we'd better to still keep below notes to mention fork/upstream trigger scopre
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #36285: [SPARK-38970][CI] Check for changes only if changes are being built / tested (not on forks)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1104666611
Can one of the admins verify this patch?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858527449
##########
.github/workflows/build_and_test.yml:
##########
@@ -393,9 +397,11 @@ jobs:
sparkr:
needs: [configure-jobs, precondition]
+ # Run scheduled jobs only with JDK 17
+ # Run regular jobs only if sparkr changes exist
Review Comment:
better also mentioned for `Apache Spark only` and `for commit in both Apache Spark and forked repository`
##########
.github/workflows/build_and_test.yml:
##########
@@ -140,11 +145,10 @@ jobs:
build:
name: "Build modules (${{ format('{0}, {1} job', needs.configure-jobs.outputs.branch, needs.configure-jobs.outputs.type) }}): ${{ matrix.modules }} ${{ matrix.comment }} (JDK ${{ matrix.java }}, ${{ matrix.hadoop }}, ${{ matrix.hive }})"
needs: [configure-jobs, precondition]
- # Run scheduled jobs for Apache Spark only
- # Run regular jobs for commit in both Apache Spark and forked repository
+ # Run for all scheduled jobs
+ # Run for all other jobs only when changes exist
Review Comment:
I think the original note still valid in here, and more useful. WDYT?
##########
.github/workflows/build_and_test.yml:
##########
@@ -140,11 +145,10 @@ jobs:
build:
name: "Build modules (${{ format('{0}, {1} job', needs.configure-jobs.outputs.branch, needs.configure-jobs.outputs.type) }}): ${{ matrix.modules }} ${{ matrix.comment }} (JDK ${{ matrix.java }}, ${{ matrix.hadoop }}, ${{ matrix.hive }})"
needs: [configure-jobs, precondition]
- # Run scheduled jobs for Apache Spark only
- # Run regular jobs for commit in both Apache Spark and forked repository
+ # Run for all scheduled jobs
+ # Run for all other jobs only when changes exist
if: >-
- (github.repository == 'apache/spark' && needs.configure-jobs.outputs.type == 'scheduled')
- || (needs.configure-jobs.outputs.type == 'regular' && fromJson(needs.precondition.outputs.required).build == 'true')
+ needs.configure-jobs.outputs.type == 'scheduled' || fromJson(needs.precondition.outputs.required).build == 'true'
Review Comment:
better to keep regular condition in here (even it's redundant now), it would be double check if somebody add new type configure-jobs in future.
##########
.github/workflows/build_and_test.yml:
##########
@@ -576,6 +582,7 @@ jobs:
java-11-17:
needs: [configure-jobs, precondition]
+ # Run regular jobs only if changes exist
Review Comment:
better also mentioned for `commit in both Apache Spark and forked repository`
##########
.github/workflows/build_and_test.yml:
##########
@@ -48,6 +48,11 @@ jobs:
configure-jobs:
name: Configure jobs
runs-on: ubuntu-20.04
+ # All other jobs in this workflow depend on this job,
+ # so the entire workflow is skipped when these conditions evaluate to false:
+ # Run all jobs for Apache Spark repository
+ # Run only non-scheduled jobs for forked repositories
Review Comment:
Good notes, but we better to still keep below notes to mention fork/upstream trigger scopre
##########
.github/workflows/build_and_test.yml:
##########
@@ -284,12 +288,12 @@ jobs:
pyspark:
needs: [configure-jobs, precondition]
- # Run PySpark coverage scheduled jobs for Apache Spark only
- # Run scheduled jobs with JDK 17 in Apache Spark
- # Run regular jobs for commit in both Apache Spark and forked repository
+ # Run PySpark coverage scheduled jobs
+ # Run scheduled jobs only with JDK 17
+ # Run regular jobs only if pyspark changes exist
Review Comment:
same
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36285:
URL: https://github.com/apache/spark/pull/36285#issuecomment-1106288464
@Yikun The expression `github.event.type` does not exist for scheduled events. The only content of `event` is the `schedule` field.
In fact, the payload for that event is not even documented: https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads
But there is `github.event_name`: https://docs.github.com/en/actions/learn-github-actions/contexts#github-context
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on a diff in pull request #36285: [SPARK-38970][CI] Skip build-and-test workflow on forks when scheduled
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on code in PR #36285:
URL: https://github.com/apache/spark/pull/36285#discussion_r858996994
##########
.github/workflows/build_and_test.yml:
##########
@@ -140,11 +145,10 @@ jobs:
build:
name: "Build modules (${{ format('{0}, {1} job', needs.configure-jobs.outputs.branch, needs.configure-jobs.outputs.type) }}): ${{ matrix.modules }} ${{ matrix.comment }} (JDK ${{ matrix.java }}, ${{ matrix.hadoop }}, ${{ matrix.hive }})"
needs: [configure-jobs, precondition]
- # Run scheduled jobs for Apache Spark only
- # Run regular jobs for commit in both Apache Spark and forked repository
+ # Run for all scheduled jobs
+ # Run for all other jobs only when changes exist
Review Comment:
I have reverted and amended the original comment.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org