[LIVY-972] Add GitHub Action for CI image by dacort · Pull Request #393 · apache/livy

dacort · 2023-03-14T04:38:59Z

What changes were proposed in this pull request?

Adds Several GitHub Workflows
- CI image builder
- Unit tests for git push
- Integration tests for PRs

How was this patch tested?

Manual validation in the course of the pull request and new GitHub Actions workflows.

ksumit

Thanks for taking this up.

…g as root. actions/runner#449

[1] apache/iotdb#3121

codecov-commenter · 2023-03-15T23:20:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.25%. Comparing base (45e07fe) to head (01b12d4).
⚠️ Report is 62 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #393      +/-   ##
============================================
- Coverage     68.37%   68.25%   -0.12%     
+ Complexity      849      847       -2     
============================================
  Files           103      103              
  Lines          5989     5989              
  Branches        911      911              
============================================
- Hits           4095     4088       -7     
- Misses         1330     1340      +10     
+ Partials        564      561       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ksumit

Would it make sense to merge integration-tests.yaml and unit-tests.yaml and use some sort of flag to use one vs the other. We could also avoid building the repository two times (though I don't think that's a big problem since the builds are happening in parallel and probably not adding to the runtime of the overall pipelines). Thoughts?

ksumit · 2023-03-21T00:01:43Z

+        id: docker_build
+        uses: docker/build-push-action@v4
+        with:
+          push: true


this is where we could change the logic to false if it's a PR build?

ksumit · 2023-03-21T00:15:13Z

+    container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+    strategy:
+      matrix:
+        spark_version: ["2.4", "3.0"]


I think we can add the unit-test vs integration-test logic here.

@dacort I wouldn't want to hold this PR for minor things. If you feel comfortable with the current state, I would push it as it is (something better than nothing) and then iterate on the improvements later on. Let me know what you think.

dacort · 2023-03-21T19:35:20Z

Would it make sense to merge integration-tests.yaml and unit-tests.yaml

I wasn't quite sure if there was an easy way to do that. Can definitely look more into it, but ideas welcome.

ksumit · 2023-03-23T00:18:43Z

Would it make sense to merge integration-tests.yaml and unit-tests.yaml

I wasn't quite sure if there was an easy way to do that. Can definitely look more into it, but ideas welcome.

Here is an example that i found on https://docs.github.com/en/actions/learn-github-actions/contexts

name: CI
on: push
jobs:
  prod-check:
    if: ${{ github.ref == 'refs/heads/main' }}
    runs-on: ubuntu-latest
    steps:
      - run: echo "Deploying to production server on branch $GITHUB_REF"

gyogal · 2023-04-06T12:32:02Z

Thanks a lot for working on this @dacort! I agree with @ksumit's points and if you feel like this is good to go as is, we could merge it to restore CI functionality for the project.
Post-merge would it be possible to run this for PRs received recently where the builds were not kicked off (such as #396)?

dacort · 2023-04-07T23:27:50Z

Thanks @gyogal - Yea, @ksumit I think for now we should merge this. I imagine the PR workflow and unit-test workflow will diverge in the future so would rather get this merged and iterate.

@gyogal re: run the checks retroactively, I don't think it'll happen automatically but if folks push another commit it should.

lmccay

+1

gyogal · 2023-04-11T12:27:45Z

@dacort Thank you for your work on this! I have merged your commit and pre-commit checks are looking good now. There are two minor issues I noticed (maybe these are just temporary and will go away after a few builds):

codecov seems to report an unrealistic drop in coverage on PRs (Merging ... will decrease coverage by 40.09%.)
post-commit unit tests are reporting failures in org.apache.livy.rsc.TestSparkClient

Again, many thanks for adding these workflows because it unblocks PRs and is of great help for the project!

dacort · 2023-04-11T14:25:43Z

@gyogal Yep I noticed that too as I work on #392. I initially thought it was related to my changes, but then noticed the failures on the main branch after this PR got merged in. I'm not quite sure what's going on as the unit tests ran fine in the PR.

I'm also guessing the codecov might be related to the unit tests not running but that's just a theory at this point.

* Add GitHub Action for CI image * Update push action * Add sample unit test * Add matrix based on spark version * Update spark version to be strings for proper comparison * Fix syntax * Add maven cache and integration tests * Just use actions/cache * Be more explicty with pom locations because container build is running as root. actions/runner#449 * Add MAVEN_OPTS to avoid timeouts[1] and ignore livy in m2 cache [1] apache/iotdb#3121 * Update cache paths * Add codecov

dacort added 3 commits March 13, 2023 21:36

Add GitHub Action for CI image

b01cbea

Update push action

f711d4d

Add sample unit test

ec2987d

ksumit reviewed Mar 14, 2023

View reviewed changes

Comment thread .github/workflows/build-ci-image.yaml Outdated

Comment thread .github/workflows/unit-tests.yaml Outdated

Add matrix based on spark version

52a27ac

dacort changed the title ~~Add GitHub Action for CI image~~ [LIVY-972] Add GitHub Action for CI image Mar 14, 2023

dacort added 8 commits March 14, 2023 14:59

Update spark version to be strings for proper comparison

bdde004

Fix syntax

9f3b1b0

Add maven cache and integration tests

abd1c6b

Just use actions/cache

a4739ac

Be more explicty with pom locations because container build is runnin…

fa91ae9

…g as root. actions/runner#449

Add MAVEN_OPTS to avoid timeouts[1] and ignore livy in m2 cache

b4b178b

[1] apache/iotdb#3121

Update cache paths

96aae63

Add codecov

01b12d4

dacort marked this pull request as ready for review March 20, 2023 18:13

ksumit reviewed Mar 21, 2023

View reviewed changes

lmccay approved these changes Apr 8, 2023

View reviewed changes

gyogal approved these changes Apr 8, 2023

View reviewed changes

gyogal merged commit da246ff into master Apr 8, 2023

dacort deleted the feature/github-actions branch April 10, 2023 16:16

Uh oh!

Conversation

dacort commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

ksumit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ksumit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ksumit Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ksumit Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

ksumit Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

dacort commented Mar 21, 2023

Uh oh!

ksumit commented Mar 23, 2023

Uh oh!

gyogal commented Apr 6, 2023

Uh oh!

dacort commented Apr 7, 2023

Uh oh!

lmccay left a comment

Choose a reason for hiding this comment

Uh oh!

gyogal commented Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dacort commented Apr 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dacort commented Mar 14, 2023 •

edited

Loading

codecov-commenter commented Mar 15, 2023 •

edited

Loading

gyogal commented Apr 11, 2023 •

edited

Loading