Skip to content
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

[KYUUBI #5789] Flink engine kyuubi.session.engine.flink.fetch.timeout parameter on the server is not effective #5790

Closed
wants to merge 1 commit into from

Conversation

junjiem
Copy link
Contributor

@junjiem junjiem commented Nov 30, 2023

🔍 Description

Issue References 🔗

This pull request fixes #5789

Describe Your Solution 🔧

First initialize the server 'kyuubi.session.engine.flink.fetch.timeout' parameters, if the server parameter does not exist, it is set to unlimited duration; And then obtain the client 'kyuubi.session.engine.flink.fetch.timeout' parameters, if the client parameters use the server does not exist.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklists

📝 Author Self Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review
  • I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

  • Pull request title is okay.
  • No license issues.
  • Milestone correctly set?
  • Test coverage is ok
  • Assignees are selected.
  • Minimum number of approvals
  • No changes are requested

@junjiem junjiem changed the title fix flink engine kyuubi.session.engine.flink.fetch.timeout parameter … [KYUUBI #5789] Flink engine kyuubi.session.engine.flink.fetch.timeout parameter on the server is not effective bug Nov 30, 2023
@junjiem junjiem changed the title [KYUUBI #5789] Flink engine kyuubi.session.engine.flink.fetch.timeout parameter on the server is not effective bug [KYUUBI #5789] Flink engine kyuubi.session.engine.flink.fetch.timeout parameter on the server is not effective Nov 30, 2023
@yaooqinn yaooqinn requested a review from link3280 November 30, 2023 06:39
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (19ae399) 61.36% compared to head (925ac25) 61.38%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5790      +/-   ##
============================================
+ Coverage     61.36%   61.38%   +0.01%     
  Complexity       23       23              
============================================
  Files           607      607              
  Lines         35935    35935              
  Branches       4936     4936              
============================================
+ Hits          22052    22059       +7     
+ Misses        11496    11491       -5     
+ Partials       2387     2385       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@link3280 link3280 added this to the v1.9.0 milestone Nov 30, 2023
Copy link
Contributor

@link3280 link3280 left a comment

Choose a reason for hiding this comment

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

@junjiem LGTM. I'll merge it when the lights turn green. Thanks!

@link3280 link3280 closed this in fc3a215 Nov 30, 2023
link3280 pushed a commit that referenced this pull request Nov 30, 2023
… parameter on the server is not effective

# 🔍 Description
## Issue References 🔗

This pull request fixes #5789

## Describe Your Solution 🔧

First initialize the server 'kyuubi.session.engine.flink.fetch.timeout' parameters, if the server parameter does not exist, it is set to unlimited duration; And then obtain the client 'kyuubi.session.engine.flink.fetch.timeout' parameters, if the client parameters use the server does not exist.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [x] Milestone correctly set?
- [x] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

Closes #5790 from junjiem/fix-5789.

Closes #5789

925ac25 [junjie.miao] fix flink engine kyuubi.session.engine.flink.fetch.timeout parameter on the server is not effective bug

Authored-by: junjie.miao <[email protected]>
Signed-off-by: Paul Lin <[email protected]>
(cherry picked from commit fc3a215)
Signed-off-by: Paul Lin <[email protected]>
@link3280
Copy link
Contributor

Merged to master and 1.8. Thanks!

@junjiem
Copy link
Contributor Author

junjiem commented Dec 1, 2023

@link3280 thank

@pan3793 pan3793 modified the milestones: v1.9.0, v1.8.1 Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants