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

Improve CI with cancel & concurrency & paths filter #2175

Closed
wants to merge 16 commits into from

Conversation

yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Mar 18, 2022

Why are the changes needed?

Better ASF GHA resource usage, see more https://cwiki.apache.org/confluence/display/BUILDS/GitHub+Actions+status

  • cancel-in-progress enabled. w/ this, a previous build can be canceled if new commit(s) have arrived
  • style check w/o install. currently, the style check takes 8~10min to finish because an install step before it. now, it shall return within a minute. note that, this might 'fail' itself when we introduce a new module and others depend on it, but it's ok to verify by the log whether a PR ok to be merged.
  • dependency check runs only when pom.xml updates. this flow now runs only pom changes as it is the only way that may cause dependency changes.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

Canceled: https://github.com/apache/incubator-kyuubi/runs/5604809742?check_suite_focus=true

image

  • Run test locally before make a pull request

@yaooqinn yaooqinn added this to the v1.6.0 milestone Mar 18, 2022
@github-actions github-actions bot added the kind:infra license, community building, project builds, asf infra related, etc. label Mar 18, 2022
@yaooqinn yaooqinn changed the title Improve CI with cancel & concurrency Improve CI with cancel & concurrency & paths filter Mar 18, 2022
@yaooqinn yaooqinn self-assigned this Mar 18, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2022

Codecov Report

Merging #2175 (33777bc) into master (db98799) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2175      +/-   ##
============================================
- Coverage     61.50%   61.45%   -0.05%     
+ Complexity      124       69      -55     
============================================
  Files           325      327       +2     
  Lines         15598    15659      +61     
  Branches       1999     2005       +6     
============================================
+ Hits           9593     9624      +31     
- Misses         5190     5214      +24     
- Partials        815      821       +6     
Impacted Files Coverage Δ
...rg/apache/kyuubi/engine/trino/TrinoStatement.scala 65.06% <0.00%> (-4.82%) ⬇️
.../org/apache/kyuubi/engine/hive/HiveSQLEngine.scala 75.00% <0.00%> (-2.78%) ⬇️
...ache/kyuubi/engine/flink/FlinkProcessBuilder.scala 71.95% <0.00%> (-2.74%) ⬇️
...ain/scala/org/apache/kyuubi/engine/EngineRef.scala 82.52% <0.00%> (-2.63%) ⬇️
...mon/src/main/scala/org/apache/kyuubi/Logging.scala 39.72% <0.00%> (-1.37%) ⬇️
...apache/kyuubi/engine/hive/HiveProcessBuilder.scala 47.61% <0.00%> (ø)
...ache/kyuubi/engine/hive/operation/GetColumns.scala 100.00% <0.00%> (ø)
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 96.22% <0.00%> (+0.02%) ⬆️
...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala 85.34% <0.00%> (+2.58%) ⬆️
...i/engine/hive/operation/HiveOperationManager.scala 45.45% <0.00%> (+4.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db98799...33777bc. Read the comment docs.

@yaooqinn
Copy link
Member Author

# Well, sometimes when we introduce a new module, it may 'fail' the style check for missing
# dependency we just create for other module to inherit.
# We can ignore this failure and merge the PR, if we see the style check pass on the new module
# or via a local quick verification.
Copy link
Member

Choose a reason for hiding this comment

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

nit: indent

@yaooqinn
Copy link
Member Author

thanks, merged to master

pan3793 added a commit that referenced this pull request Feb 9, 2023
### _Why are the changes needed?_

Since #2175, the previous running CI jobs will be canceled on the same branch or PR to save CI resources, this PR keeps the behavior for PR but doesn't cancel the running CI jobs when commit to the stable branch.

Ref: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-a-fallback-value

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate

I pushed an empty commit to verify the behavior does not change in PR, and I need to merge this to master first, then merge another PR immediately to verify the rest behaviors.

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4285 from pan3793/ci-con.

Closes #4285

3aa7f42 [Cheng Pan] empty
4156eb1 [Cheng Pan] Do not cancel CI jobs on stable branch

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:build kind:infra license, community building, project builds, asf infra related, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants