You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Attila Doroszlai <ad...@hortonworks.com> on 2017/08/10 13:47:49 UTC
Review Request 61557: AMBARI-21701. Add check for import from
relocated packages
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61557/
-----------------------------------------------------------
Review request for Ambari, Jonathan Hurley and Sebastian Toader.
Bugs: AMBARI-21701
https://issues.apache.org/jira/browse/AMBARI-21701
Repository: ambari
Description
-------
Consider `org.apache.hadoop.metrics2.sink.relocated.*` to be [illegal import](http://checkstyle.sourceforge.net/config_imports.html#IllegalImport), in addition to the default `sun.*`.
Diffs
-----
ambari-server/checkstyle.xml fb314bccc19f6a2e687c070620e06beec7d465fa
Diff: https://reviews.apache.org/r/61557/diff/1/
Testing
-------
```
# introduce "relocated" import
$ git revert --no-commit 1433ecff108df159f71961e23a34d248e1684c85
# compile with dependencies (same as before)
$ mvn -am -pl ambari-server -DskipTests clean test
...
[INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ ambari-server ---
[INFO] Compiling 792 source files to ambari-server/target/test-classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project ambari-server: Compilation failure: Compilation failure:
[ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[39,71] package org.apache.hadoop.metrics2.sink.relocated.google.common.collect does not exist
[ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[114,34] cannot find symbol
[ERROR] symbol: variable Sets
[ERROR] location: class org.apache.ambari.server.checks.AbstractCheckDescriptorTest
...
[INFO] BUILD FAILURE
# compile with installed artifacts
$ mvn -am -pl ambari-metrics/ambari-metrics-common,ambari-serviceadvisor,ambari-views clean install
$ mvn -pl ambari-server -DskipTests clean test
...
[INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
[INFO] Starting audit...
[ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:39:1: Import from illegal package - org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets. [IllegalImport]
Audit done.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
# back to clean state
$ git revert --abort
$ mvn -pl ambari-server -DskipTests clean test
...
[INFO] BUILD SUCCESS
$ mvn -am -pl ambari-server -DskipTests clean test
...
[INFO] BUILD SUCCESS
```
Thanks,
Attila Doroszlai
Re: Review Request 61557: AMBARI-21701. Add check for import from
relocated packages
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61557/#review182591
-----------------------------------------------------------
Ship it!
Ship It!
- Jonathan Hurley
On Aug. 10, 2017, 9:47 a.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61557/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2017, 9:47 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Sebastian Toader.
>
>
> Bugs: AMBARI-21701
> https://issues.apache.org/jira/browse/AMBARI-21701
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Consider `org.apache.hadoop.metrics2.sink.relocated.*` to be [illegal import](http://checkstyle.sourceforge.net/config_imports.html#IllegalImport), in addition to the default `sun.*`.
>
>
> Diffs
> -----
>
> ambari-server/checkstyle.xml fb314bccc19f6a2e687c070620e06beec7d465fa
>
>
> Diff: https://reviews.apache.org/r/61557/diff/1/
>
>
> Testing
> -------
>
> ```
> # introduce "relocated" import
> $ git revert --no-commit 1433ecff108df159f71961e23a34d248e1684c85
>
> # compile with dependencies (same as before)
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ ambari-server ---
> [INFO] Compiling 792 source files to ambari-server/target/test-classes
> [INFO] -------------------------------------------------------------
> [ERROR] COMPILATION ERROR :
> [INFO] -------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project ambari-server: Compilation failure: Compilation failure:
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[39,71] package org.apache.hadoop.metrics2.sink.relocated.google.common.collect does not exist
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[114,34] cannot find symbol
> [ERROR] symbol: variable Sets
> [ERROR] location: class org.apache.ambari.server.checks.AbstractCheckDescriptorTest
> ...
> [INFO] BUILD FAILURE
>
> # compile with installed artifacts
> $ mvn -am -pl ambari-metrics/ambari-metrics-common,ambari-serviceadvisor,ambari-views clean install
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:39:1: Import from illegal package - org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets. [IllegalImport]
> Audit done.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
>
> # back to clean state
> $ git revert --abort
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
>
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>
Re: Review Request 61557: AMBARI-21701. Add check for import from
relocated packages
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61557/#review182593
-----------------------------------------------------------
Ship it!
Ship It!
- Sebastian Toader
On Aug. 10, 2017, 3:47 p.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61557/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2017, 3:47 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Sebastian Toader.
>
>
> Bugs: AMBARI-21701
> https://issues.apache.org/jira/browse/AMBARI-21701
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Consider `org.apache.hadoop.metrics2.sink.relocated.*` to be [illegal import](http://checkstyle.sourceforge.net/config_imports.html#IllegalImport), in addition to the default `sun.*`.
>
>
> Diffs
> -----
>
> ambari-server/checkstyle.xml fb314bccc19f6a2e687c070620e06beec7d465fa
>
>
> Diff: https://reviews.apache.org/r/61557/diff/1/
>
>
> Testing
> -------
>
> ```
> # introduce "relocated" import
> $ git revert --no-commit 1433ecff108df159f71961e23a34d248e1684c85
>
> # compile with dependencies (same as before)
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ ambari-server ---
> [INFO] Compiling 792 source files to ambari-server/target/test-classes
> [INFO] -------------------------------------------------------------
> [ERROR] COMPILATION ERROR :
> [INFO] -------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project ambari-server: Compilation failure: Compilation failure:
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[39,71] package org.apache.hadoop.metrics2.sink.relocated.google.common.collect does not exist
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[114,34] cannot find symbol
> [ERROR] symbol: variable Sets
> [ERROR] location: class org.apache.ambari.server.checks.AbstractCheckDescriptorTest
> ...
> [INFO] BUILD FAILURE
>
> # compile with installed artifacts
> $ mvn -am -pl ambari-metrics/ambari-metrics-common,ambari-serviceadvisor,ambari-views clean install
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:39:1: Import from illegal package - org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets. [IllegalImport]
> Audit done.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
>
> # back to clean state
> $ git revert --abort
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
>
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>
Re: Review Request 61557: AMBARI-21701. Add check for import from
relocated packages
Posted by Attila Doroszlai <ad...@hortonworks.com>.
> On Aug. 10, 2017, 4:17 p.m., Jonathan Hurley wrote:
> > ambari-server/checkstyle.xml
> > Lines 27 (patched)
> > <https://reviews.apache.org/r/61557/diff/1/?file=1794867#file1794867line27>
> >
> > Are there legit cases where sun.* is needed (such as with LDAP or SSL stuff?)
Not currently:
```
$ fgrep -r 'import sun.' ambari-server/src/????/java || echo 'No sun.*'
No sun.*
```
Also, this setting is already in effect, since the check is present without any custom property value.
Anyone adding such stuff later can remove `sun`.
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61557/#review182586
-----------------------------------------------------------
On Aug. 10, 2017, 3:47 p.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61557/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2017, 3:47 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Sebastian Toader.
>
>
> Bugs: AMBARI-21701
> https://issues.apache.org/jira/browse/AMBARI-21701
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Consider `org.apache.hadoop.metrics2.sink.relocated.*` to be [illegal import](http://checkstyle.sourceforge.net/config_imports.html#IllegalImport), in addition to the default `sun.*`.
>
>
> Diffs
> -----
>
> ambari-server/checkstyle.xml fb314bccc19f6a2e687c070620e06beec7d465fa
>
>
> Diff: https://reviews.apache.org/r/61557/diff/1/
>
>
> Testing
> -------
>
> ```
> # introduce "relocated" import
> $ git revert --no-commit 1433ecff108df159f71961e23a34d248e1684c85
>
> # compile with dependencies (same as before)
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ ambari-server ---
> [INFO] Compiling 792 source files to ambari-server/target/test-classes
> [INFO] -------------------------------------------------------------
> [ERROR] COMPILATION ERROR :
> [INFO] -------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project ambari-server: Compilation failure: Compilation failure:
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[39,71] package org.apache.hadoop.metrics2.sink.relocated.google.common.collect does not exist
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[114,34] cannot find symbol
> [ERROR] symbol: variable Sets
> [ERROR] location: class org.apache.ambari.server.checks.AbstractCheckDescriptorTest
> ...
> [INFO] BUILD FAILURE
>
> # compile with installed artifacts
> $ mvn -am -pl ambari-metrics/ambari-metrics-common,ambari-serviceadvisor,ambari-views clean install
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:39:1: Import from illegal package - org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets. [IllegalImport]
> Audit done.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
>
> # back to clean state
> $ git revert --abort
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
>
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>
Re: Review Request 61557: AMBARI-21701. Add check for import from
relocated packages
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61557/#review182586
-----------------------------------------------------------
ambari-server/checkstyle.xml
Lines 27 (patched)
<https://reviews.apache.org/r/61557/#comment258537>
Are there legit cases where sun.* is needed (such as with LDAP or SSL stuff?)
- Jonathan Hurley
On Aug. 10, 2017, 9:47 a.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61557/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2017, 9:47 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Sebastian Toader.
>
>
> Bugs: AMBARI-21701
> https://issues.apache.org/jira/browse/AMBARI-21701
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Consider `org.apache.hadoop.metrics2.sink.relocated.*` to be [illegal import](http://checkstyle.sourceforge.net/config_imports.html#IllegalImport), in addition to the default `sun.*`.
>
>
> Diffs
> -----
>
> ambari-server/checkstyle.xml fb314bccc19f6a2e687c070620e06beec7d465fa
>
>
> Diff: https://reviews.apache.org/r/61557/diff/1/
>
>
> Testing
> -------
>
> ```
> # introduce "relocated" import
> $ git revert --no-commit 1433ecff108df159f71961e23a34d248e1684c85
>
> # compile with dependencies (same as before)
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ ambari-server ---
> [INFO] Compiling 792 source files to ambari-server/target/test-classes
> [INFO] -------------------------------------------------------------
> [ERROR] COMPILATION ERROR :
> [INFO] -------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project ambari-server: Compilation failure: Compilation failure:
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[39,71] package org.apache.hadoop.metrics2.sink.relocated.google.common.collect does not exist
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:[114,34] cannot find symbol
> [ERROR] symbol: variable Sets
> [ERROR] location: class org.apache.ambari.server.checks.AbstractCheckDescriptorTest
> ...
> [INFO] BUILD FAILURE
>
> # compile with installed artifacts
> $ mvn -am -pl ambari-metrics/ambari-metrics-common,ambari-serviceadvisor,ambari-views clean install
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [ERROR] ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java:39:1: Import from illegal package - org.apache.hadoop.metrics2.sink.relocated.google.common.collect.Sets. [IllegalImport]
> Audit done.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
>
> # back to clean state
> $ git revert --abort
> $ mvn -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] BUILD SUCCESS
>
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>