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

SOLR-16956: fix not being able to stop a solr by port number #1880

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

epugh
Copy link
Contributor

@epugh epugh commented Sep 1, 2023

https://issues.apache.org/jira/browse/SOLR-16956

Description

In doing some other testing, I noticed that bin/solr stop -p 7574 didn't work, I get back the error message:

Found 2 Solr nodes running! Must either specify a port using -p or -all to stop all Solr nodes on this host.

Solution

Set the PROVIDED_SOLR_PORT to the SOLR_PORT.

Tests

extended bats tests

@epugh epugh requested a review from HoustonPutman September 1, 2023 10:46
@epugh
Copy link
Contributor Author

epugh commented Sep 1, 2023

@HoustonPutman didn't you recently look at some port stuff? COuld you review this bug fix?

@epugh
Copy link
Contributor Author

epugh commented Sep 1, 2023

I tested on Branch_9x, and this issue is only on Main.....

solr/bin/solr Outdated Show resolved Hide resolved
@cpoerschke cpoerschke changed the title fix not being able to stop a solr by port number SOLR-16956: fix not being able to stop a solr by port number Sep 1, 2023
Co-authored-by: Christine Poerschke <[email protected]>
@epugh
Copy link
Contributor Author

epugh commented Sep 2, 2023

Thanks @cpoerschke for the feedback... one thing I noticed in the BATS docs is that it suggests running shellcheck. That isn't actually something I've ever done, and I wonder if it's the type of thing that should be run as part of precommit to make sure we keep both our bin/solr script and the various bats tests following best practices?

@cpoerschke
Copy link
Contributor

... it suggests running shellcheck. That isn't actually something I've ever done, and I wonder if it's the type of thing that should be run as part of precommit ...

I've not yet run shellcheck either but running as part of precommit too sounds like a good idea.

@epugh
Copy link
Contributor Author

epugh commented Sep 5, 2023

I am going to merge this without an entry in Changes.txt since it's a bugfix in 10, that doesn't appear to exist in 9...

@epugh epugh merged commit 0eef652 into apache:main Sep 5, 2023
Copy link
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

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.

4 participants