Add static-checks Github Action by tejaswini-imply · Pull Request #13347 · apache/druid

tejaswini-imply · 2022-11-10T05:25:48Z

We are moving away from the Travis CI/CD pipeline to GitHub Actions. As the first step, this PR adds static-checks GitHub action (triggered on any pull request activity) to run all the below steps:

animal sniffer checks
checkstyle
enforcer checks
forbidden api checks
pmd checks
spotbugs checks
license checks
license checks for hadoop 3
script checks
analyze dependencies
analyze hadoop 3 dependencies
(openjdk11) strict compilation

Optimizations implemented:

Caching maven dependencies.
Running checks parallelly.
Concurrency - single job per PR at a time.

FrankChen021 · 2022-11-10T14:50:48Z

.github/workflows/static-checks.yml

+
+name: Static Checks CI
+on:
+  pull_request:


I think we also need this GitHub Action to run on each commit on master and release branch.

e.g.

on: push: branches: [ master ]

BTW, why does the GitHub Action not run for this PR?

it's possible that Infra needs to enable actions for this repo?

Seems like it. This runs on a PR raised in the fork repo.

https://the-asf.slack.com/archives/CBX4TSBQ8/p1668410098261039 - We need to merge the first PR before it can get triggered.

tejaswini-imply · 2022-11-14T09:40:38Z

Here is the Static Checks CI GitHub actions on a forked repo - https://github.com/tejaswini-imply/druid/actions/runs/3442505350

imply-elliott

Generally looks great for a basic pipeline we can iterate upon!

imply-elliott · 2022-11-16T00:39:34Z

.github/workflows/static-checks.yml

+          cache: 'maven'
+      - run: |
+          echo 'Running Maven install...' &&
+          MAVEN_OPTS='-Xmx3000m' \


Let's move this to a env variable so it's set the same way as others and universally applied:

env: MAVEN_OPTS: -Xmx3000m

imply-elliott · 2022-11-16T00:41:26Z

.github/scripts/setup_generate_license.sh

+
+sudo apt-get update && sudo apt-get install python3 -y
+curl https://bootstrap.pypa.io/pip/3.5/get-pip.py | sudo -H python3
+../../check_test_suite.py && travis_terminate 0 || echo 'Continuing setup'


What's this reference to travis_terminate here? I saw this in my tests too and I don't think this will work in Actions?

imply-elliott · 2022-11-16T00:45:05Z

.github/workflows/static-checks.yml

+          java-version: '8'
+          cache: 'maven'
+      - run: |-
+          MAVEN_OPTS='-Xmx3000m' ${MVN} ${MAVEN_SKIP} dependency:analyze -DoutputXML=true -DignoreNonCompile=true -DfailOnWarning=true ${{ matrix.HADOOP_PROFILE }} ||


We can pull out MAVEN_OPTS here when set as a global env variable.

imply-elliott · 2022-11-16T00:45:42Z

.github/workflows/static-checks.yml

+        # errorprone requires JDK 11
+        # Strict compilation requires more than 2 GB
+      - run: |
+          MAVEN_OPTS='-Xmx3000m' ${MVN} clean -DstrictCompile compile test-compile --fail-at-end ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS}


We can pull out MAVEN_OPTS here when set as a global env variable.

imply-elliott · 2022-11-16T00:46:30Z

.github/workflows/static-checks.yml

+    needs: [build]
+    steps:
+      - uses: actions/checkout@v3
+      - name: setup java 8


Do we expect to only run some of these checks with java 8, or should they eventually be run for all supported versions?

Maybe eventually, This is the existing flow with Travis checks.

tejaswini-imply · 2022-11-16T10:35:36Z

Thanks for the review @imply-elliott, I have addressed your comments in the latest commit. PTAL.

imply-elliott

Thanks for addressing my comments!

tejaswini-imply added 13 commits November 3, 2022 11:43

Adds static-checks github action

5753f85

bug fixes

0b8b0a4

bug fixes

07f634a

adds maven install step

fec96a0

update permissions on scripts folder

746f3f4

fix license check errors

70b9570

attempt for parallelization

972aecc

fix concurrency

9243a50

fix naming

38c1ec2

remove intellij inspections to add in different CI pipeline

a0dfb2a

minimize naming, add new lines

e307b58

Merge branch 'master' into add-static_checks_github_action

e9f5ca7

setting hadoop profile through matrix

d583df7

FrankChen021 reviewed Nov 10, 2022

View reviewed changes

also runs on push triggers to master and release branches

ce090b6

cryptoe added the Area - Dev For items related to the project itself, like dev docs and checklists, but not CI label Nov 13, 2022

imply-elliott reviewed Nov 16, 2022

View reviewed changes

changes on review

c57fad9

imply-elliott approved these changes Nov 16, 2022

View reviewed changes

abhishekagarwal87 approved these changes Nov 17, 2022

View reviewed changes

abhishekagarwal87 merged commit 8e9e46b into apache:master Nov 17, 2022

kfaraz added this to the 25.0 milestone Nov 21, 2022

Comments

Conversation

tejaswini-imply commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tejaswini-imply commented Nov 14, 2022

Uh oh!

imply-elliott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tejaswini-imply commented Nov 16, 2022

Uh oh!

imply-elliott left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants