Skip to content

[ZEPPELIN-1409] Refactor RAT build on Travis.CI configuration#1401

Closed
lresende wants to merge 2 commits intoapache:masterfrom
lresende:build
Closed

[ZEPPELIN-1409] Refactor RAT build on Travis.CI configuration#1401
lresende wants to merge 2 commits intoapache:masterfrom
lresende:build

Conversation

@lresende
Copy link
Member

@lresende lresende commented Sep 4, 2016

What is this PR for?

Create a specific build for checking license compliance with RAT
and avoid running these checks on every build that compose the
PR build.
As for normal development builds, this follows the same pattern
used for maven tests, RAT is enabled to run by default, but now
there is support disabling it with -DskipRat.
Travis CI will run RAT once, on the RAT build, and disable RAT
checks on all other build profiles.

What type of PR is it?

[Enhancement]

What is the Jira issue?

@lresende
Copy link
Member Author

lresende commented Sep 4, 2016

One of the builds failed with the following, which seems not related

  • should provide onclick method *** FAILED ***
    The code passed to eventually never returned normally. Attempted 1 times over 1.164197794 seconds. Last failure message: 0 was not equal to 1. (AbstractAngularElemTest.scala:72)

@lresende
Copy link
Member Author

lresende commented Sep 4, 2016

@bzz @Leemoonsoo Please review

@lresende lresende changed the title [SPARK-17392] Refactor RAT build on Travis.CI configuration [ZEPPELIN-1409] Refactor RAT build on Travis.CI configuration Sep 4, 2016
@@ -34,6 +34,10 @@ addons:

matrix:
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the contribution. Shell we have better comment here?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Leemoonsoo updated.

<version>${jetty.version}</version>
</dependency>

<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Is this related to RAT?

@bzz
Copy link
Member

bzz commented Sep 5, 2016

I think having a separate profile it is reasonable approach for CI. Though I also think there is value in keeping the default behaviour for local Dev workflow and make RAT check on by default.

It might be like this for some time ZEPPELIN-278

In order to communicate the value of such fix better, I would suggest linking to JIRA/here the logs of CI builds that have failed due to RAT issue as well (like current master, #1400, etc).

Create a specific build for checking license compliance with RAT
and avoid running these checks on every build that compose the
PR build.
Following the same pattern used for maven tests, enable RAT to run
by default, but support disabling it with -DskipRat. Travis CI will
run RAT once, on the RAT build, and disable RAT checks on all other
build profiles.
@lresende
Copy link
Member Author

lresende commented Sep 5, 2016

@bzz this has now been updated to follow the same pattern used for maven tests, enable RAT to run by default, but support disabling it with -DskipRat. Travis CI will run RAT once, on the RAT build, and disable RAT checks on all other build profiles.

@corneadoug
Copy link
Contributor

Then how about we use -DskipRat on all building profiles, and have a profile that run only RAT without building?

Build time is pretty long, so it would be nice to separate some easy check that do not need compilation or specific settings separated, we would keep the CI feedback loop shorter and it would also be easier to know what failed without going to the log.

It could probably also be applied to things like Style Check or Unit Tests later

@lresende
Copy link
Member Author

lresende commented Sep 6, 2016

@corneadoug This is exactly what is implemented on this pr.

@corneadoug
Copy link
Contributor

@lresende My bad, when I first read I understood that RAT would be included in one profile that would still build everything. (just like we do with the -DskipTests)

@felixcheung
Copy link
Member

LGTM

@bzz
Copy link
Member

bzz commented Sep 6, 2016

Thank you @lresende ! Looks great to me, merging to master as a hotfix, if there is no further discussion.

CI fails on a single profile with un-related flaky test

[INFO] Zeppelin: Server ................................... FAILURE [03:39 min]

Failed tests: 
  ZeppelinSparkClusterTest.zRunTest:204 expected:<FINISHED> but was:<ERROR>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants