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

Prevent server start if port in use #1394

Closed
wants to merge 2 commits into from

Conversation

yakkomajuri
Copy link
Contributor

Closes #1160

  • Adds a check to see if a port is already in use in the specified host and prevent sql-server from starting if that's the case
  • Adds a test for this behavior

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@oscarbatori oscarbatori requested a review from zachmu March 7, 2021 16:14
@timsehn
Copy link
Sponsor Contributor

timsehn commented Mar 8, 2021

I'd love to see a bats test for this one.

@zachmu
Copy link
Member

zachmu commented Mar 8, 2021

Hi @yakkomajuri ,

Can you add a test for this in the bats directory? You need to install bats to run the tests in that directory.

Also we need you to accept the CLA before we can accept this.

@yakkomajuri
Copy link
Contributor Author

So sorry, I find the project super cool but I'm not comfortable signing the CLA.

Saw it on HN and immediately went into the codebase before checking the license, my apologies!

@yakkomajuri yakkomajuri closed this Mar 9, 2021
@zachmu zachmu reopened this Apr 6, 2021
@zachmu
Copy link
Member

zachmu commented Apr 6, 2021

Hi @yakkomajuri,

We have eliminated the CLA requirement if you still want to contribute this. Might need to open a fresh PR to spur GitHub into realizing that the CLA is gone.

@yakkomajuri
Copy link
Contributor Author

Hey @zachmu! Sounds good, I'll try to give bats a look over the weekend and submit a new PR

@timsehn
Copy link
Sponsor Contributor

timsehn commented Apr 8, 2021

Thanks for prodding us on the CLA. We realized it was dumb because of this PR :-)

@timsehn
Copy link
Sponsor Contributor

timsehn commented Apr 21, 2021

Pinging here :-)

@yakkomajuri
Copy link
Contributor Author

Hey @timsehn ! Thanks for the ping - I'm happy to resubmit the PR but not sure I'll get to bats too soon - been super super busy, so apologies for the delay!

@timsehn
Copy link
Sponsor Contributor

timsehn commented Apr 21, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dolt sql-server runs on occupied port
4 participants