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-17066: Switch Http2SolrClient away from coreURLs #2255

Conversation

gerlowskija
Copy link
Contributor

@gerlowskija gerlowskija commented Feb 8, 2024

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

Description

SOLR-17066 introduced a withDefaultCollection builder method that can be used in lieu of providing a core URL as a "base". (Core URLs are not ideal as they prevent clients from making core-agnostic requests.)

However, Solr's own client usage still needs modified to use this new builder method.

Solution

This commit migrates Http2SolrClient away from core URLs and towards using withDefaultCollection. This (along with #2254) are the last vestiges of core URLs in Solr!

Tests

Tests continue to pass. No additional tests as this is "just" a refactor.

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)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.

Providing a core URL as a SolrClient's "base URL" prevents it from
communicating with other cores or making core-agnostic API requests
(e.g. node healthcheck, list cores, etc.)

This commit migrates all Http2SolrClient usage away from core URLs.
Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -89,9 +90,16 @@ public void runImpl(CommandLine cli) throws Exception {
}

public void runCommand(String baseUrl, String root, String credentials) throws IOException {
if (URLUtil.isBaseUrl(baseUrl)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is interesting.. I wonder if "url" is a good name for this parameter? I like the check... --collection-url? -collection-url.. though we dont have any other commands with a - in the name.... at least we have the check.

@gerlowskija gerlowskija merged commit bdf0b2f into apache:main Feb 13, 2024
3 of 4 checks passed
@gerlowskija gerlowskija deleted the SOLR-17066-jetty-http-remove-core-urls-pt-1 branch February 13, 2024 14:06
gerlowskija added a commit that referenced this pull request Feb 13, 2024
Providing a core URL as a SolrClient's "base URL" prevents it from
communicating with other cores or making core-agnostic API requests
(e.g. node healthcheck, list cores, etc.)

This commit migrates all Http2SolrClient usage away from core URLs.
jdyer1 added a commit to jdyer1/solr that referenced this pull request Feb 20, 2024
jdyer1 added a commit to jdyer1/solr that referenced this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants