Skip to content

Conversation

@ChinmaySKulkarni
Copy link
Contributor

No description provided.

@ChinmaySKulkarni
Copy link
Contributor Author

Please review @yanxinyi @jpisaac @gjacoby126 @kadirozde @twdsilva

@twdsilva
Copy link
Contributor

twdsilva commented Oct 3, 2020

+1, nice work thanks for fixing all these scenarios.

@ChinmaySKulkarni
Copy link
Contributor Author

Thanks for the review @twdsilva!
@kadirozde @gjacoby126 @jpisaac @yanxinyi as discussed offline, I have written a design doc outlining my approach: PHOENIX-6142 Design Doc. Please take a look. It would be great to get your reviews as well. The push I did after Thomas's review does not contain any code changes. They are just extra comments.

@stoty
Copy link
Contributor

stoty commented Oct 16, 2020

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 30s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ 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.
_ 4.x Compile Tests _
+1 💚 mvninstall 26m 21s 4.x passed
+1 💚 compile 0m 54s 4.x passed
+1 💚 checkstyle 3m 2s 4.x passed
+1 💚 javadoc 0m 43s 4.x passed
+0 🆗 spotbugs 2m 50s phoenix-core in 4.x has 956 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 22m 49s the patch passed
+1 💚 compile 0m 56s the patch passed
+1 💚 javac 0m 56s the patch passed
-1 ❌ checkstyle 3m 7s phoenix-core: The patch generated 417 new + 5804 unchanged - 345 fixed = 6221 total (was 6149)
-1 ❌ whitespace 0m 0s The patch 1 line(s) with tabs.
-1 ❌ javadoc 0m 43s phoenix-core generated 1 new + 99 unchanged - 1 fixed = 100 total (was 100)
-1 ❌ spotbugs 3m 6s phoenix-core generated 1 new + 953 unchanged - 3 fixed = 954 total (was 956)
_ Other Tests _
-1 ❌ unit 125m 23s phoenix-core in the patch failed.
+1 💚 asflicense 0m 38s The patch does not generate ASF License warnings.
193m 48s
Reason Tests
FindBugs module:phoenix-core
Found reliance on default encoding in org.apache.phoenix.util.ViewUtil.findImmediateRelatedViews(Table, byte[], byte[], byte[], PTable$LinkType, long):in org.apache.phoenix.util.ViewUtil.findImmediateRelatedViews(Table, byte[], byte[], byte[], PTable$LinkType, long): String.getBytes() At ViewUtil.java:[line 287]
Failed junit tests phoenix.end2end.PhoenixRuntimeIT
phoenix.end2end.ToCharFunctionIT
phoenix.end2end.DeleteIT
phoenix.end2end.IndexToolForNonTxGlobalIndexIT
phoenix.end2end.UpsertSelectIT
phoenix.end2end.index.ImmutableIndexExtendedIT
phoenix.end2end.DropSchemaIT
phoenix.end2end.index.IndexMaintenanceIT
phoenix.end2end.OrphanViewToolIT
Subsystem Report/Notes
Docker ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #903
JIRA Issue PHOENIX-6142
Optional Tests dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
uname Linux c35418f930ab 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev/phoenix-personality.sh
git revision 4.x / 264310b
Default Java Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstyle https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
whitespace https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/whitespace-tabs.txt
javadoc https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/diff-javadoc-javadoc-phoenix-core.txt
spotbugs https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html
unit https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Results https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/testReport/
Max. process+thread count 6414 (vs. ulimit of 30000)
modules C: phoenix-core U: phoenix-core
Console output https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-903/6/console
versions git=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ChinmaySKulkarni
Copy link
Contributor Author

None of the tests that failed in the build above are failing locally.
Re-running the preCommit manually to get another run.

Copy link
Contributor

@jpisaac jpisaac left a comment

Choose a reason for hiding this comment

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

+1 LGTM, except a request for a couple of more tests.

@ChinmaySKulkarni
Copy link
Contributor Author

Thanks for the review @jpisaac , I will modify the test and commit after a successful preCommit

@ChinmaySKulkarni ChinmaySKulkarni merged commit afda3c1 into apache:4.x Oct 21, 2020
@ChinmaySKulkarni ChinmaySKulkarni deleted the PHOENIX-6142 branch October 21, 2020 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants