You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/01/10 15:06:47 UTC
[GitHub] [phoenix] virajjasani opened a new pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
virajjasani opened a new pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757656154
Thanks @yanxinyi for the review. @stoty has already created [PHOENIX-6234](https://issues.apache.org/jira/browse/PHOENIX-6234) for replacing guava with thirdparty in 4.x.
Until then, the only enforcement we can see is to restrict old logging framework: #1083
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757657123
> this may be useful if this runs before the tests, (and on 4.x.)
Yes, it does catch this before running tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757663659
Sounds good. (good to know it wasn't me who skipped the 4.x backport)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757674069
Attaching sample build failure:
```
[WARNING] Rule 2: de.skuzzle.enforcer.restrictimports.rule.RestrictImports failed with message:
Banned imports detected in TEST code:
Reason: Use commons lang 3
in file: org/apache/phoenix/end2end/AlterTableWithViewsIT.java
org.apache.commons.lang.StringUtils (Line: 41, Matched by: org.apache.commons.lang.**)
Analysis took 0 seconds
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Apache Phoenix 5.1.0-SNAPSHOT ...................... SUCCESS [ 1.701 s]
[INFO] Phoenix Hbase 2.4.0 compatibility .................. SUCCESS [ 3.718 s]
[INFO] Phoenix Hbase 2.3.0 compatibility .................. SUCCESS [ 0.946 s]
[INFO] Phoenix Hbase 2.2.1 compatibility .................. SUCCESS [ 0.893 s]
[INFO] Phoenix Hbase 2.1.6 compatibility .................. SUCCESS [ 0.620 s]
[INFO] Phoenix Core ....................................... FAILURE [ 3.595 s]
[INFO] Phoenix - Pherf .................................... SKIPPED
[INFO] Phoenix Client Parent .............................. SKIPPED
[INFO] Phoenix Client ..................................... SKIPPED
[INFO] Phoenix Server ..................................... SKIPPED
[INFO] Phoenix - Tracing Web Application .................. SKIPPED
[INFO] Phoenix Assembly ................................... SKIPPED
[INFO] phoenix-tools ...................................... SKIPPED
[INFO] Phoenix Client Embedded 5.1.0-SNAPSHOT ............. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 12.127 s
[INFO] Finished at: 2021-01-11T12:57:06+05:30
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M3:enforce (banned-illegal-imports) on project phoenix-core: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed. -> [Help 1]
[ERROR]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757656611
On master, this is somewhat redundant, as dependency-check will fail the build in these cases.
That was never backported to 4.x (but eventually should be)
However, as dependency-check runs AFTER the tests, and the tests still fail more oftent than not, this may be useful if this runs before the tests, (and on 4.x.)
The commons-logging and commons-lang stuff definitely applies to 4.x
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757659384
Alright, Thanks @stoty @yanxinyi . Let me create that backport Jira then. Better not change it for now in 4.x. Sounds good?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani edited a comment on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757656154
Thanks @yanxinyi for the review.
> Do we have a plan to clean the code and do the same thing at the 4.x branch?
@stoty has already created [PHOENIX-6234](https://issues.apache.org/jira/browse/PHOENIX-6234) for replacing guava with thirdparty in 4.x.
Until then, the only enforcement we can see is to restrict old logging framework: #1083
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757658696
In that it probably not a regression, I just never got around to backporting that cleanup to 4.x
I'll leave it to your discretion wheter you want to that for ticket.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty edited a comment on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty edited a comment on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757658696
In that it probably not a regression, I just never got around to backporting that cleanup to 4.x
I'll leave it to your discretion wheter you want to that for this ticket.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani edited a comment on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757659384
Alright, Thanks @stoty @yanxinyi . Better not change it for now in 4.x as it is this way for quite some time (more than 2 yr).
We can revisit this after 4.16 release. Sounds good?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757924531
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 9s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 16m 7s | master passed |
| +1 :green_heart: | compile | 1m 31s | master passed |
| +1 :green_heart: | javadoc | 1m 17s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 17s | the patch passed |
| +1 :green_heart: | compile | 1m 36s | the patch passed |
| +1 :green_heart: | javac | 1m 36s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 20s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 269m 1s | root in the patch failed. |
| +1 :green_heart: | asflicense | 0m 30s | The patch does not generate ASF License warnings. |
| | | 304m 9s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.index.GlobalMutableNonTxIndexIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1082 |
| Optional Tests | dupname asflicense javac javadoc unit xml compile |
| uname | Linux fd5ecc1bcde0 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / fe0b3ca |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/3/artifact/yetus-general-check/output/patch-unit-root.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/3/testReport/ |
| Max. process+thread count | 5740 (vs. ulimit of 30000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/3/console |
| versions | git=2.7.4 maven=3.3.9 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757608405
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 15m 42s | master passed |
| +1 :green_heart: | compile | 1m 32s | master passed |
| +1 :green_heart: | javadoc | 1m 17s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 32s | the patch passed |
| +1 :green_heart: | javac | 1m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 18s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 177m 54s | root in the patch failed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 212m 35s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1082 |
| Optional Tests | dupname asflicense javac javadoc unit xml compile |
| uname | Linux 381bf3490809 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / fe0b3ca |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/2/artifact/yetus-general-check/output/patch-unit-root.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/2/testReport/ |
| Max. process+thread count | 5755 (vs. ulimit of 30000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1082/2/console |
| versions | git=2.7.4 maven=3.3.9 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757666075
My bad, in fact we can use [1.1.0](https://mvnrepository.com/artifact/de.skuzzle.enforcer/restrict-imports-enforcer-rule/1.1.0) instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757658125
4.x has many references to commons-lang. Let me fix them and update #1083 quickly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757660446
On master, commons-lang was upgraded as part of [PHOENIX-4880](https://issues.apache.org/jira/browse/PHOENIX-4880)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757653437
LGTM +1 for the master branch. However, do we have a plan to do the same thing for the 4.x? I remember that we don't have enforcement for certain important. For example, we have `import com.google.common.collect.ImmutableMap;` and `import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableMap;`. Do we have a plan to clean the code and do the same thing at the 4.x branch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757657252
Does 4.x still have commons-lang ? That is probably a regression, and you may want to fix it in #1083 and add the rule.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty closed pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
stoty closed pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1082: PHOENIX-6309 : Use maven enforcer plugin to ban imports from illegal packages (as per project guidelines)
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1082:
URL: https://github.com/apache/phoenix/pull/1082#issuecomment-757666652
While copying from other project, I somehow missed looking at latest versions. Let me update PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org