Skip to content

Conversation

@wForget
Copy link
Member

@wForget wForget commented Dec 21, 2023

Why are the changes needed?

close #5244

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

Was this patch authored or co-authored using generative AI tooling?

No

Closes #5251 from lsm1/branch-kyuubi-5244.

Closes #5244

bcadaa5 [senmiaoliu] rename
a6c9277 [senmiaoliu] fix style
1f38fa7 [senmiaoliu] fix style
3ff57ff [senmiaoliu] Make engineAliveMaxFailCount configurable

Authored-by: senmiaoliu senmiaoliu@trip.com
Signed-off-by: Shaoyun Chen csy@apache.org
(cherry picked from commit 9ea0a1b)

…urable

### _Why are the changes needed?_

close apache#5244
### _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/master/contributing/code/testing.html#running-tests) locally before make a pull request

### _Was this patch authored or co-authored using generative AI tooling?_

No

Closes apache#5251 from lsm1/branch-kyuubi-5244.

Closes apache#5244

bcadaa5 [senmiaoliu] rename
a6c9277 [senmiaoliu] fix style
1f38fa7 [senmiaoliu] fix style
3ff57ff [senmiaoliu] Make engineAliveMaxFailCount configurable

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Shaoyun Chen <csy@apache.org>
(cherry picked from commit 9ea0a1b)
@pan3793 pan3793 changed the title [KYUUBI #5244][Improvement][1.8] Make engineAliveMaxFailCount configurable [KYUUBI #5244][1.8] Make engineAliveMaxFailCount configurable Dec 21, 2023
@wForget wForget removed their assignment Dec 21, 2023
@pan3793 pan3793 added this to the v1.8.1 milestone Dec 21, 2023
val ENGINE_ALIVE_MAX_FAILURES: ConfigEntry[Int] =
buildConf("kyuubi.session.engine.alive.max.failures")
.doc("The maximum number of failures allowed for the engine.")
.version("1.8.0")
Copy link
Member

Choose a reason for hiding this comment

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

due to the merging issue, this is invalid, we need a followup PR to fix it as 1.8.1

pan3793 pushed a commit that referenced this pull request Dec 21, 2023
### _Why are the changes needed?_

close #5244
### _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/master/contributing/code/testing.html#running-tests) locally before make a pull request

### _Was this patch authored or co-authored using generative AI tooling?_

No

Closes #5251 from lsm1/branch-kyuubi-5244.

Closes #5244

bcadaa5 [senmiaoliu] rename
a6c9277 [senmiaoliu] fix style
1f38fa7 [senmiaoliu] fix style
3ff57ff [senmiaoliu] Make engineAliveMaxFailCount configurable

Authored-by: senmiaoliu <senmiaoliutrip.com>
Signed-off-by: Shaoyun Chen <csyapache.org>
(cherry picked from commit 9ea0a1b)

Closes #5888 from wForget/dev-1.8_backport_5251.

Closes #5244

cf9c725 [senmiaoliu] [KYUUBI #5244][Improvement] Make engineAliveMaxFailCount configurable

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
@pan3793
Copy link
Member

pan3793 commented Dec 21, 2023

Merged to 1.8

@pan3793 pan3793 closed this Dec 21, 2023
pan3793 pushed a commit that referenced this pull request Dec 21, 2023
…sion

# 🔍 Description
## Issue References 🔗

This pull request fixes #5244

## Describe Your Solution 🔧

As discussed in #5888 (comment), we need to correct the ENGINE_ALIVE_MAX_FAILURES  conf version.

## 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
- [x] 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
- [ ] 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

**Be nice. Be informative.**

Closes #5889 from wForget/KYUUBI-5244-FOLLOWUP.

Closes #5244

a785449 [wforget] [KYUUBI #5244][FOLLOWUP] Corrected ENGINE_ALIVE_MAX_FAILURES conf version

Authored-by: wforget <643348094@qq.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this pull request Dec 21, 2023
…sion

# 🔍 Description
## Issue References 🔗

This pull request fixes #5244

## Describe Your Solution 🔧

As discussed in #5888 (comment), we need to correct the ENGINE_ALIVE_MAX_FAILURES  conf version.

## 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
- [x] 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
- [ ] 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

**Be nice. Be informative.**

Closes #5889 from wForget/KYUUBI-5244-FOLLOWUP.

Closes #5244

a785449 [wforget] [KYUUBI #5244][FOLLOWUP] Corrected ENGINE_ALIVE_MAX_FAILURES conf version

Authored-by: wforget <643348094@qq.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit 7b7e17b)
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this pull request Mar 21, 2024
…nf version

# 🔍 Description
## Issue References 🔗

This pull request fixes apache#5244

## Describe Your Solution 🔧

As discussed in apache#5888 (comment), we need to correct the ENGINE_ALIVE_MAX_FAILURES  conf version.

## 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
- [x] 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
- [ ] 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

**Be nice. Be informative.**

Closes apache#5889 from wForget/KYUUBI-5244-FOLLOWUP.

Closes apache#5244

a785449 [wforget] [KYUUBI apache#5244][FOLLOWUP] Corrected ENGINE_ALIVE_MAX_FAILURES conf version

Authored-by: wforget <643348094@qq.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
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.

4 participants