Skip to content

PHOENIX-6357 Change all command line tools to use the fixed commons-c… - #1184

Closed
stoty wants to merge 1 commit into
apache:masterfrom
stoty:PHOENIX-6357
Closed

PHOENIX-6357 Change all command line tools to use the fixed commons-c…#1184
stoty wants to merge 1 commit into
apache:masterfrom
stoty:PHOENIX-6357

Conversation

@stoty

@stotystoty commented Mar 25, 2021

Copy link
Copy Markdown
Contributor

…li constructor

@stoty

stoty commented Mar 25, 2021

Copy link
Copy Markdown
ContributorAuthor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 11sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe 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 _
+0 🆗mvndep4m 50sMaven dependency ordering for branch
+1 💚mvninstall12m 40smaster passed
+0hbaserecompile24m 32sHBase recompiled.
+1 💚compile2m 6smaster passed
+1 💚checkstyle1m 41smaster passed
+1 💚javadoc1m 30smaster passed
+0 🆗spotbugs3m 16sphoenix-core in master has 956 extant spotbugs warnings.
+0 🆗spotbugs0m 49sphoenix-pherf in master has 42 extant spotbugs warnings.
+0 🆗spotbugs0m 44sphoenix-tools in master has 3 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 11sMaven dependency ordering for patch
+1 💚mvninstall9m 52sthe patch passed
+0hbaserecompile16m 52sHBase recompiled.
+1 💚compile2m 5sthe patch passed
+1 💚javac2m 5sthe patch passed
+1 💚checkstyle1m 13sphoenix-core: The patch generated 0 new + 2577 unchanged - 1 fixed = 2577 total (was 2578)
+1 💚checkstyle0m 16sThe patch passed checkstyle in phoenix-pherf
+1 💚checkstyle0m 13sThe patch passed checkstyle in phoenix-tools
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc1m 29sthe patch passed
+1 💚spotbugs5m 27sthe patch passed
_ Other Tests _
-1 ❌unit112m 31sphoenix-core in the patch failed.
-1 ❌unit8m 37sphoenix-pherf in the patch failed.
+1 💚unit3m 4sphoenix-tools in the patch passed.
+1 💚asflicense1m 28sThe patch does not generate ASF License warnings.
192m 50s
ReasonTests
Failed junit testsphoenix.end2end.CsvBulkLoadToolIT
phoenix.pherf.PherfMainIT
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1184
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux a7fb216012c6 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / f9532bd
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/1/artifact/yetus-general-check/output/patch-unit-phoenix-pherf.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/1/testReport/
Max. process+thread count11676 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-pherf phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/1/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty

stoty commented Mar 25, 2021

Copy link
Copy Markdown
ContributorAuthor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 13sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe 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 _
+0 🆗mvndep4m 54sMaven dependency ordering for branch
+1 💚mvninstall12m 49smaster passed
+0hbaserecompile24m 37sHBase recompiled.
+1 💚compile2m 6smaster passed
+1 💚checkstyle1m 39smaster passed
+1 💚javadoc1m 32smaster passed
+0 🆗spotbugs3m 19sphoenix-core in master has 956 extant spotbugs warnings.
+0 🆗spotbugs0m 49sphoenix-pherf in master has 42 extant spotbugs warnings.
+0 🆗spotbugs0m 43sphoenix-tools in master has 3 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for patch
+1 💚mvninstall9m 57sthe patch passed
+0hbaserecompile16m 57sHBase recompiled.
+1 💚compile2m 6sthe patch passed
+1 💚javac2m 6sthe patch passed
-1 ❌checkstyle1m 12sphoenix-core: The patch generated 25 new + 2540 unchanged - 39 fixed = 2565 total (was 2579)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc1m 28sthe patch passed
+1 💚spotbugs5m 24sthe patch passed
_ Other Tests _
-1 ❌unit124m 25sphoenix-core in the patch failed.
-1 ❌unit10m 40sphoenix-pherf in the patch failed.
+1 💚unit3m 5sphoenix-tools in the patch passed.
+1 💚asflicense1m 30sThe patch does not generate ASF License warnings.
207m 9s
ReasonTests
Failed junit testsphoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT
phoenix.pherf.PherfMainIT
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1184
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux 21341f6bf058 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / f9532bd
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/artifact/yetus-general-check/output/patch-unit-phoenix-pherf.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/testReport/
Max. process+thread count10758 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-pherf phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/2/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty

stoty commented Apr 6, 2021

Copy link
Copy Markdown
ContributorAuthor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec7m 3sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe 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 _
+0 🆗mvndep4m 55sMaven dependency ordering for branch
+1 💚mvninstall13m 4smaster passed
+0hbaserecompile25m 3sHBase recompiled.
+1 💚compile2m 7smaster passed
+1 💚checkstyle1m 39smaster passed
+1 💚javadoc1m 32smaster passed
+0 🆗spotbugs3m 19sphoenix-core in master has 956 extant spotbugs warnings.
+0 🆗spotbugs0m 53sphoenix-pherf in master has 51 extant spotbugs warnings.
+0 🆗spotbugs0m 42sphoenix-tools in master has 3 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for patch
+1 💚mvninstall10m 10sthe patch passed
+0hbaserecompile17m 15sHBase recompiled.
+1 💚compile2m 7sthe patch passed
+1 💚javac2m 7sthe patch passed
-1 ❌checkstyle1m 13sphoenix-core: The patch generated 25 new + 2539 unchanged - 39 fixed = 2564 total (was 2578)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc1m 32sthe patch passed
+1 💚spotbugs5m 24sthe patch passed
_ Other Tests _
-1 ❌unit124m 4sphoenix-core in the patch failed.
-1 ❌unit9m 39sphoenix-pherf in the patch failed.
+1 💚unit2m 59sphoenix-tools in the patch passed.
+1 💚asflicense1m 32sThe patch does not generate ASF License warnings.
212m 16s
ReasonTests
Failed junit testsphoenix.pherf.PherfMainIT
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1184
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux c7b11abf5297 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 21599ac
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/artifact/yetus-general-check/output/patch-unit-phoenix-pherf.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/testReport/
Max. process+thread count10064 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-pherf phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/3/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty
PHOENIX-6357 Change all command line tools to use the fixed commons-c…
97d0c84
…li constructor
note that this also removes the partial double doublequote workaround, as it is no longer needed
@stoty

stoty commented May 7, 2021

Copy link
Copy Markdown
ContributorAuthor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 10sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe 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 _
+0 🆗mvndep4m 55sMaven dependency ordering for branch
+1 💚mvninstall13m 29smaster passed
+0hbaserecompile26m 18sHBase recompiled.
+1 💚compile2m 25smaster passed
+1 💚checkstyle1m 56smaster passed
+1 💚javadoc2m 3smaster passed
+0 🆗spotbugs3m 44sphoenix-core in master has 959 extant spotbugs warnings.
+0 🆗spotbugs1m 2sphoenix-pherf in master has 51 extant spotbugs warnings.
+0 🆗spotbugs0m 51sphoenix-tools in master has 3 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 14sMaven dependency ordering for patch
+1 💚mvninstall10m 19sthe patch passed
+0hbaserecompile17m 51sHBase recompiled.
+1 💚compile2m 22sthe patch passed
+1 💚javac2m 22sthe patch passed
-1 ❌checkstyle1m 13sphoenix-core: The patch generated 25 new + 2540 unchanged - 39 fixed = 2565 total (was 2579)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc1m 37sthe patch passed
+1 💚spotbugs6m 24sthe patch passed
_ Other Tests _
-1 ❌unit127m 47sphoenix-core in the patch failed.
-1 ❌unit8m 53sphoenix-pherf in the patch failed.
+1 💚unit3m 27sphoenix-tools in the patch passed.
+1 💚asflicense1m 27sThe patch does not generate ASF License warnings.
214m 58s
ReasonTests
Failed junit testsphoenix.end2end.ConcurrentUpsertsWithoutIndexedColsIT
phoenix.end2end.UpsertSelectIT
phoenix.pherf.workload.mt.tenantoperation.TenantViewOperationWorkloadIT
phoenix.pherf.PherfMainIT
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1184
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux 557331b84312 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 5265164
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/artifact/yetus-general-check/output/patch-unit-phoenix-pherf.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/testReport/
Max. process+thread count8864 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-pherf phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1184/4/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

virajjasani
virajjasani reviewed May 9, 2021
View reviewed changes

@virajjasanivirajjasani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one doubt, rest of changes look good

Comment threadphoenix-core/src/main/java/org/apache/phoenix/mapreduce/AbstractBulkLoadTool.java
if (quotedSchemaName) {
schemaName = schemaName.substring(1,schemaName.length() - 1);
}

@virajjasanivirajjasaniMay 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change is not required because cli's parsing issue with double quote is no longer an issue. Is it correct?

@stotystotyMay 9, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, this was for the double doublequote hack to workaround CLI-254, which we patch in phoenix-thirdparty now.

@stotystoty closed this May 10, 2021
@stoty
stoty deleted the PHOENIX-6357 branch July 29, 2021 14:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@virajjasanivirajjasanivirajjasani approved these changes
@yanxinyiyanxinyiAwaiting requested review from yanxinyi
@gjacoby126gjacoby126Awaiting requested review from gjacoby126

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stoty@virajjasani