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 #4515] Capturing process id for kyuubi server launched using run command #6374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Madhukar98
Copy link

…run command

🔍 Description

Issue References 🔗

This pull request fixes # #4515

Describe Your Solution 🔧

Currently, the process ID (PID) for a Kyuubi server launched using bin/kyuubi run is not being captured. This leads to the bin/kyuubi status command incorrectly reporting that the server is not running.
Furthermore, if the bin/kyuubi run command is initiated at the launch of a Docker container, subsequent attempts to run bin/kyuubi start will fail due to the error "ports already occupied".
To resolve these issues and ensure synchronization between the status of the Kyuubi server whether it's launched in the foreground with bin/kyuubi run or in the background with bin/kyuubi start, two changes have been made:

  1. A get_pid function has been created to capture the PID of the process launched using the run command.
  2. A check has been added before starting a Kyuubi server to ensure that it's not already running in a different mode.

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 ⚰️

bin/kyuubi run -> Launches kyuubi server
bin/kyuubi status -> Kyuubi is not running
bin/kyuubi start -> Fails with port already occupied

Behavior With This Pull Request 🎉

bin/kyuubi run -> Launches kyuubi server
bin/kyuubi status -> Kyuubi is running (PID)
bin/kyuubi start -> Kyuubi running as process PID. Stop it first.

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.44%. Comparing base (78e104d) to head (bbee33a).
Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6374      +/-   ##
============================================
- Coverage     58.45%   58.44%   -0.02%     
  Complexity       24       24              
============================================
  Files           653      653              
  Lines         39881    39895      +14     
  Branches       5481     5482       +1     
============================================
+ Hits          23313    23315       +2     
- Misses        14075    14080       +5     
- Partials       2493     2500       +7     

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

@cxzl25 cxzl25 changed the title [KYUUBI #4515] Capturing process id for kyuubi server launched using … [KYUUBI #4515] Capturing process id for kyuubi server launched using run command May 9, 2024
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.

None yet

2 participants