You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Sid Wagle <sw...@hortonworks.com> on 2016/11/29 21:14:57 UTC
Re: Review Request 54002: AMBARI-18966 Add check to ensure we do not
have @Transactional annotations on private methods
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54002/#review157330
-----------------------------------------------------------
ambari-server/checkstyle.xml (line 16)
<https://reviews.apache.org/r/54002/#comment227880>
IMO we should fail compilation since this is important from developer point of view and can be easily corrected at development phase.
- Sid Wagle
On Nov. 23, 2016, 8:43 p.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54002/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2016, 8:43 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Myroslav Papirkovskyy, Sebastian Toader, and Sid Wagle.
>
>
> Bugs: AMBARI-18966
> https://issues.apache.org/jira/browse/AMBARI-18966
>
>
> Repository: ambari
>
>
> Description
> -------
>
> * Custom checkstyle check is implemented in `utility` module
> * `checkstyle:check` goal added to `test` phase of `ambari-server` module
> * Currently only warning is printed for offending methods, could be simply changed (in `ambari-server/checkstyle.xml`) to error to make build fail in such case
> * Limitation: it only checks methods; while the annotation may be specified at the class level, it is not used on classes in Ambari (possible future enhancement).
>
>
> Diffs
> -----
>
> ambari-project/pom.xml 7f273eb
> ambari-server/checkstyle.xml PRE-CREATION
> ambari-server/pom.xml 69ab9d0
> utility/pom.xml 2febb83
> utility/src/main/java/org/apache/ambari/checkstyle/AvoidTransactionalOnPrivateMethodsCheck.java PRE-CREATION
> utility/src/main/resources/checkstyle_packages.xml PRE-CREATION
> utility/src/test/java/org/apache/ambari/checkstyle/AvoidTransactionalOnPrivateMethodsCheckTest.java PRE-CREATION
> utility/src/test/resources/org/apache/ambari/checkstyle/InputTransactionalOnPrivateMethods.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54002/diff/
>
>
> Testing
> -------
>
> ```
> $ mvn -pl utility clean install
> ...
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.212 sec - in org.apache.ambari.checkstyle.AvoidTransactionalOnPrivateMethodsCheckTest
> ...
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 13 licence.
> ...
> [INFO] BUILD SUCCESS
> ```
>
> and
>
> ```
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 5190 licence.
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java:375: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java:444: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java:484: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java:958: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1494: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1525: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1555: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java:184: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java:956: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java:453: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/services/RetryUpgradeActionService.java:192: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java:1315: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java:1565: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java:2787: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> Audit done.
> ...
> [INFO] BUILD SUCCESS
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>
Re: Review Request 54002: AMBARI-18966 Add check to ensure we do not
have @Transactional annotations on private methods
Posted by Sumit Mohanty <sm...@hortonworks.com>.
> On Nov. 29, 2016, 9:14 p.m., Sid Wagle wrote:
> > ambari-server/checkstyle.xml, line 16
> > <https://reviews.apache.org/r/54002/diff/3/?file=1569574#file1569574line16>
> >
> > IMO we should fail compilation since this is important from developer point of view and can be easily corrected at development phase.
+1 to failing the build. Assuming we have no errors now. Its a very useful change.
- Sumit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54002/#review157330
-----------------------------------------------------------
On Nov. 23, 2016, 8:43 p.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54002/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2016, 8:43 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Myroslav Papirkovskyy, Sebastian Toader, and Sid Wagle.
>
>
> Bugs: AMBARI-18966
> https://issues.apache.org/jira/browse/AMBARI-18966
>
>
> Repository: ambari
>
>
> Description
> -------
>
> * Custom checkstyle check is implemented in `utility` module
> * `checkstyle:check` goal added to `test` phase of `ambari-server` module
> * Currently only warning is printed for offending methods, could be simply changed (in `ambari-server/checkstyle.xml`) to error to make build fail in such case
> * Limitation: it only checks methods; while the annotation may be specified at the class level, it is not used on classes in Ambari (possible future enhancement).
>
>
> Diffs
> -----
>
> ambari-project/pom.xml 7f273eb
> ambari-server/checkstyle.xml PRE-CREATION
> ambari-server/pom.xml 69ab9d0
> utility/pom.xml 2febb83
> utility/src/main/java/org/apache/ambari/checkstyle/AvoidTransactionalOnPrivateMethodsCheck.java PRE-CREATION
> utility/src/main/resources/checkstyle_packages.xml PRE-CREATION
> utility/src/test/java/org/apache/ambari/checkstyle/AvoidTransactionalOnPrivateMethodsCheckTest.java PRE-CREATION
> utility/src/test/resources/org/apache/ambari/checkstyle/InputTransactionalOnPrivateMethods.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54002/diff/
>
>
> Testing
> -------
>
> ```
> $ mvn -pl utility clean install
> ...
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.212 sec - in org.apache.ambari.checkstyle.AvoidTransactionalOnPrivateMethodsCheckTest
> ...
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 13 licence.
> ...
> [INFO] BUILD SUCCESS
> ```
>
> and
>
> ```
> $ mvn -am -pl ambari-server -DskipTests clean test
> ...
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 5190 licence.
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java:375: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java:444: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java:484: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java:958: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1494: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1525: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java:1555: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java:184: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java:956: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java:453: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/services/RetryUpgradeActionService.java:192: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java:1315: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java:1565: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> [WARN] ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java:2787: @Transactional should not be used on private methods [AvoidTransactionalOnPrivateMethods]
> Audit done.
> ...
> [INFO] BUILD SUCCESS
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>