Skip to content
This repository has been archived by the owner on Jan 9, 2020. It is now read-only.

Fix lint-check failures and javadoc8 break #187

Merged
merged 2 commits into from
Mar 16, 2017

Conversation

ash211
Copy link

@ash211 ash211 commented Mar 15, 2017

(cherry picked from commit ed84cd0)

## What changes were proposed in this pull request?

This PR proposes to fix lint-check failures and javadoc8 break.

Few errors were introduced as below:

**lint-check failures**

```
[ERROR] src/test/java/org/apache/spark/network/TransportClientFactorySuite.java:[45,1] (imports) RedundantImport: Duplicate import to line 43 - org.apache.spark.network.util.MapConfigProvider.
[ERROR] src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java:[255,10] (modifier) RedundantModifier: Redundant 'final' modifier.
```

**javadoc8**

```
[error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/StreamingQueryProgress.java:19: error: bad use of '>'
[error]  *                   "max" -> "2016-12-05T20:54:20.827Z"  // maximum event time seen in this trigger
[error]                             ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/StreamingQueryProgress.java:20: error: bad use of '>'
[error]  *                   "min" -> "2016-12-05T20:54:20.827Z"  // minimum event time seen in this trigger
[error]                             ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/StreamingQueryProgress.java:21: error: bad use of '>'
[error]  *                   "avg" -> "2016-12-05T20:54:20.827Z"  // average event time seen in this trigger
[error]                             ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/StreamingQueryProgress.java:22: error: bad use of '>'
[error]  *                   "watermark" -> "2016-12-05T20:54:20.827Z"  // watermark used in this trigger
[error]
```

## How was this patch tested?

Manually checked as below:

**lint-check failures**

```
./dev/lint-java
Checkstyle checks passed.
```

**javadoc8**

This seems hidden in the API doc but I manually checked after removing access modifier as below:

It looks not rendering properly (scaladoc).

![2016-12-16 3 40 34](https://cloud.githubusercontent.com/assets/6477701/21255175/8df1fe6e-c3ad-11e6-8cda-ce7f76c6677a.png)

After this PR, it renders as below:

- scaladoc
  ![2016-12-16 3 40 23](https://cloud.githubusercontent.com/assets/6477701/21255135/4a11dab6-c3ad-11e6-8ab2-b091c4f45029.png)

- javadoc
  ![2016-12-16 3 41 10](https://cloud.githubusercontent.com/assets/6477701/21255137/4bba1d9c-c3ad-11e6-9b88-62f1f697b56a.png)

Author: hyukjinkwon <gurwls223@gmail.com>

Closes apache#16307 from HyukjinKwon/lint-javadoc8.

(cherry picked from commit ed84cd0)
@foxish
Copy link
Member

foxish commented Mar 15, 2017

@ash211 Looks like the import shouldn't be removed. Can we revert that particular change in a new commit?

@ash211
Copy link
Author

ash211 commented Mar 16, 2017

Hmm, looks like Travis and Jenkins both failed in the same SparkContextSuite though on different tests..

Jenkins:

- addJar can be called twice with same file in local-mode (SPARK-16787)
- addFile can be called twice with same file in local-mode (SPARK-16787)
- addJar can be called twice with same file in non-local-mode (SPARK-16787)
- addFile can be called twice with same file in non-local-mode (SPARK-16787) *** FAILED ***
  java.lang.IllegalStateException: Shutdown hooks cannot be modified during shutdown.
  at org.apache.spark.util.SparkShutdownHookManager.add(ShutdownHookManager.scala:195)
  at org.apache.spark.util.ShutdownHookManager$.addShutdownHook(ShutdownHookManager.scala:153)
  at org.apache.spark.storage.DiskBlockManager.addShutdownHook(DiskBlockManager.scala:145)
  at org.apache.spark.storage.DiskBlockManager.<init>(DiskBlockManager.scala:51)
  at org.apache.spark.storage.BlockManager.<init>(BlockManager.scala:82)
  at org.apache.spark.SparkEnv$.create(SparkEnv.scala:349)
  at org.apache.spark.SparkEnv$.createDriverEnv(SparkEnv.scala:174)
  at org.apache.spark.SparkContext.createSparkEnv(SparkContext.scala:257)
  at org.apache.spark.SparkContext.<init>(SparkContext.scala:432)
  at org.apache.spark.SparkContext.<init>(SparkContext.scala:145)
  ...

Travis

- addJar can be called twice with same file in local-mode (SPARK-16787)
- addFile can be called twice with same file in local-mode (SPARK-16787)
- addJar can be called twice with same file in non-local-mode (SPARK-16787)
- addFile can be called twice with same file in non-local-mode (SPARK-16787)
- Cancelling job group should not cause SparkContext to shutdown (SPARK-6414) *** FAILED ***
  java.lang.NullPointerException:
  at org.apache.spark.SparkContextSuite$$anonfun$14.apply$mcV$sp(SparkContextSuite.scala:303)
  at org.apache.spark.SparkContextSuite$$anonfun$14.apply(SparkContextSuite.scala:293)
  at org.apache.spark.SparkContextSuite$$anonfun$14.apply(SparkContextSuite.scala:293)
  at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22)
  at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)
  at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
  at org.scalatest.Transformer.apply(Transformer.scala:22)
  at org.scalatest.Transformer.apply(Transformer.scala:20)
  at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:166)
  at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:68)
  ...

Could this suite be full of flaky tests?

@foxish
Copy link
Member

foxish commented Mar 16, 2017

rerun unit tests please

@foxish
Copy link
Member

foxish commented Mar 16, 2017

Oh nvm, it worked now.

@ash211
Copy link
Author

ash211 commented Mar 16, 2017

And success that time! It passed the Jenkins checks but not the Travis ones (which I just restarted)

@foxish are you comfortable merging this now as an incremental improvement to the prep-for-alpha-release branch as we work through these testing issues?

@foxish foxish merged commit b139b46 into prep-for-alpha-release Mar 16, 2017
@ash211 ash211 deleted the lint-java-fix branch March 16, 2017 02:27
@foxish
Copy link
Member

foxish commented Mar 16, 2017

I think we can stop looking at travis at this point, due to the memory errors. Thanks for fixing this @ash211. Do you think we can now merge #177 ?

@ash211
Copy link
Author

ash211 commented Mar 16, 2017

Hoping so, let's continue discussion there

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants