Skip to content

[KYUUBI #4388] Limit the max rows for get nextRowSet api#4395

Closed
lightning-L wants to merge 5 commits intoapache:masterfrom
lightning-L:kyuubi-4388
Closed

[KYUUBI #4388] Limit the max rows for get nextRowSet api#4395
lightning-L wants to merge 5 commits intoapache:masterfrom
lightning-L:kyuubi-4388

Conversation

@lightning-L
Copy link
Contributor

Why are the changes needed?

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

@lightning-L
Copy link
Contributor Author

@turboFei

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2023

Codecov Report

Merging #4395 (4f4ae8e) into master (3b73e1d) will decrease coverage by 0.01%.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master    #4395      +/-   ##
============================================
- Coverage     53.74%   53.74%   -0.01%     
  Complexity       13       13              
============================================
  Files           564      564              
  Lines         30895    30907      +12     
  Branches       4164     4165       +1     
============================================
+ Hits          16605    16610       +5     
- Misses        12736    12742       +6     
- Partials       1554     1555       +1     
Impacted Files Coverage Δ
...ache/kyuubi/server/api/v1/OperationsResource.scala 72.27% <66.66%> (-0.18%) ⬇️
...apache/kyuubi/service/AbstractBackendService.scala 97.01% <80.00%> (-1.38%) ⬇️
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 97.53% <100.00%> (+0.07%) ⬆️
...ache/kyuubi/operation/KyuubiOperationManager.scala 80.00% <0.00%> (-2.67%) ⬇️
...g/apache/kyuubi/operation/BatchJobSubmission.scala 75.82% <0.00%> (-1.65%) ⬇️
...g/apache/kyuubi/session/KyuubiSessionManager.scala 94.36% <0.00%> (-0.71%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@turboFei turboFei added this to the v1.8.0 milestone Feb 22, 2023
Copy link
Member

@turboFei turboFei left a comment

Choose a reason for hiding this comment

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

LGTM , Left minor comments

@turboFei
Copy link
Member

turboFei commented Feb 22, 2023

After think twice, I think that we can set the default value to optional, to do not break the original behavior.

@lightning-L lightning-L requested a review from turboFei February 22, 2023 13:19
@turboFei
Copy link
Member

thanks, merging to master

@turboFei turboFei closed this in 27a8674 Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants