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-17483: Backport cleanup zk-related logic in bin/solr.cmd #2823

Merged

Conversation

malliaridis
Copy link
Contributor

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

Description

See #2816 for details.

Additionally, this PR also fixes a bug introduced in 7892085 that lead to "mode." form an echo message being executed as a command.

Solution

See #2816 for details.

Tests

Windows tests still pending. See SOLR-17508.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

malliaridis and others added 2 commits October 30, 2024 20:41
* Remove ZK logic from solr.cmd as it is obsolete
* Fix unreachable auth tool in solr.cmd
* Fix invalid references

(cherry picked from commit c2091d0)
The closing parentheses caused the IF to be closed before
the echo message was fully printed, leading to "mode."
being executed instead of printed.
@malliaridis malliaridis changed the title SOLR-17483: Cleanup zk-related logic in bin/solr.cmd SOLR-17483: Backport cleanup zk-related logic in bin/solr.cmd Oct 30, 2024
@epugh
Copy link
Contributor

epugh commented Nov 6, 2024

I know we want to add windwos testing, but I wouldn't let that hold this up.

@malliaridis
Copy link
Contributor Author

Since there are bugfixes included as well, I think we are good to go. There is no ETA for the Windows testing, so it should not be a blocker.

@epugh
Copy link
Contributor

epugh commented Nov 9, 2024

I just saw a bug report (with a patch!) reported to the dev list: https://issues.apache.org/jira/browse/SOLR-17551. Looking a bit closer, I wonder if we apply this PR to branch_9x and branch_9_7, then does this fix the reported bug as well, or if we only do the bug fix in branch_9_7, and leave this one to go to branch_9x?

@malliaridis
Copy link
Contributor Author

It seems to be backwards compatible, so there is probably no issues backport this to branch_9_7 as well. :)

@malliaridis malliaridis merged commit 9023e70 into apache:branch_9x Nov 9, 2024
3 of 4 checks passed
@malliaridis
Copy link
Contributor Author

I've further looked into backporting this, but it would break compatibility to other deprecated commands, which we probably don't want to mess with again. I would therefore apply the patch from https://issues.apache.org/jira/browse/SOLR-17551 instead.

@epugh
Copy link
Contributor

epugh commented Nov 9, 2024

I've further looked into backporting this, but it would break compatibility to other deprecated commands, which we probably don't want to mess with again. I would therefore apply the patch from https://issues.apache.org/jira/browse/SOLR-17551 instead.

Awesome. Could I talk you into applying it? I don't have a windows box and it's tricky enough I don't want to break things worse. Thanks for making a decision on which way to go!!!!

@malliaridis
Copy link
Contributor Author

Leaving this as a note: Mistakenly changed the commit message with vim during cherrypick. The correct issue is SOLR-17483, not SOLR-17484.

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.

2 participants