Skip to content

Build: Ensure checkstyle and all check run for Flink PRs - #3520

Closed
kbendick wants to merge 1 commit into
apache:masterfrom
kbendick:disable-quick-build-flink-with-coauthor
Closed

Build: Ensure checkstyle and all check run for Flink PRs#3520
kbendick wants to merge 1 commit into
apache:masterfrom
kbendick:disable-quick-build-flink-with-coauthor

Conversation

@kbendick

Copy link
Copy Markdown
Contributor

This PR is trying to address this comment: #3501 (comment)

It seems that checkstyle has not been running for some (or all) of the Java / Scala code outside of java-ci.yml due to -Pquick=true.

I had noticed and fixed a rogue checkstyle issue that made it in, but I thought it slipped in during the refactoring of tests as a one time thing.

@openinx suggested in this PR: #3509 that we remove the path ignores in java-ci.yaml. But I think that would negate the work of splitting up the tests and the speed built up that we have seen from that.

Co-authored-by: huzheng openinx@gmail.com

Co-authored-by: huzheng <openinx@gmail.com>
@kbendick

Copy link
Copy Markdown
ContributorAuthor

The other reason things work presently in java-ci.yml is that we have two sections related to tests and checks.

We have the one used presently, equivalent to core-tests, and then the build-checks section.

I'm going to add build-chcks into the existing code so that nothing gets in that doesn't pass CI as it should, and then prioritize a top level workflow that orchestrates all of them so when one fails, they all fail.

 core-tests:
runs-on: ubuntu-latest
strategy:
matrix:
jvm: [8, 11]
env:
SPARK_LOCAL_IP: localhost
steps:
- uses: actions/checkout@v2
- uses: actions/setup-java@v1
with:
java-version: ${{ matrix.jvm }}
- uses: actions/cache@v2
with:
path: ~/.gradle/caches
key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
restore-keys: ${{ runner.os }}-gradle
- run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- run: ./gradlew check -DsparkVersions= -DhiveVersions= -DflinkVersions= -Pquick=true -x javadoc
- uses: actions/upload-artifact@v2
if: failure()
with:
name: test logs
path: |
**/build/testlogs
build-checks:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-java@v1
with:
java-version: 8
- run: ./gradlew build -x test -x javadoc -x integrationTest

@nastra

Copy link
Copy Markdown
Contributor

@kbendick do we need to adjust the same thing in hive-ci.yml?

@kbendick

Copy link
Copy Markdown
ContributorAuthor

@kbendick do we need to adjust the same thing in hive-ci.yml?

Yes we do. If I don't add that one today (a bit under the weather), feel free to do it too.

@rdblue

Copy link
Copy Markdown
Contributor

@kbendick, I commented on the Spark PR. This should use the existing Java CI job instead of adding duplicate checks that will slow down Spark and Flink test builds.

@rdblue

Copy link
Copy Markdown
Contributor

Closing since this was just fixed by #3509.

@rdbluerdblue closed this Nov 14, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kbendick@nastra@rdblue