-
Notifications
You must be signed in to change notification settings - Fork 980
[KYUUBI #551][JDK11] Fix KyuubiSQLExceptionSuite #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @jackMintao, can you provide your git |
Codecov Report
@@ Coverage Diff @@
## master #688 +/- ##
==========================================
+ Coverage 80.49% 80.51% +0.02%
==========================================
Files 120 122 +2
Lines 4717 4738 +21
Branches 574 574
==========================================
+ Hits 3797 3815 +18
- Misses 599 601 +2
- Partials 321 322 +1
Continue to review full report at Codecov.
|
|
I will fix #689 |
|
|
|
Thanks, merged for v1.3.0. |
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html 2. If the PR is related to an issue in https://github.com/NetEase/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'. 3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'. --> ### _Why are the changes needed?_ <!-- Please clarify why the changes are needed. For instance, 1. If you add a feature, you can talk about the use case of it. 2. If you fix a bug, you can clarify why it is a bug. --> Close #550, relate to #559 #688 #689. Add `javax.xml.bind:jaxb-api` and `javax.activation:activation` since those apis has been removed by JDK 11. In GA, we saw a significant performance downgrade in the unit tests, and even with some changes like enlarging timeout, using in-memory catalog instead of hive catalog, the JDK11(~43min) tests cost 2 times than JDK8(~22min), this performance regression is tracked in #745. ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/latest/tools/testing.html#running-tests) locally before make a pull request Closes #693 from pan3793/jdk11. Closes #693 5706daf [Cheng Pan] shrink scalatest memory 6ebac68 [Cheng Pan] address comments 2b47939 [Cheng Pan] address comments 5fa74aa [Cheng Pan] disable test in kyuubi-extension-spark_3.1 9f070a4 [Cheng Pan] maven profile for jdk versions 801e4cd [Cheng Pan] jdk11 in GA e86893c [Cheng Pan] enlarge YarnCluster timeout c880edd [Cheng Pan] minor 38d2775 [Cheng Pan] enlarge engine crash timeout a945ade [Cheng Pan] use in-memory catalog in EngineRefSuite 51604d7 [Cheng Pan] enlarge ENGINE_INIT_TIMEOUT 7baeb96 [Cheng Pan] minicluster a7e4bdf [Cheng Pan] GA matrix b29aa2f [Cheng Pan] license for jaxb 4283777 [Cheng Pan] Add dependency jaxb-api c5b0e58 [Cheng Pan] [GA] Enable JDK 11 in GA Lead-authored-by: Cheng Pan <[email protected]> Co-authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]>
Why are the changes needed?
Make JDK11 work step by step, in this PR, we fix the issue in
KyuubiSQLExceptionSuiteand report a new one in MetricsSystemSuiteclose #559 and file a new ticket to track #689
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request