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 HttpSolrClient away from core URLs, pt 1 #2173

Merged

Conversation

gerlowskija
Copy link
Contributor

@gerlowskija gerlowskija commented Jan 2, 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 PR tackles this for the necessary production (i.e. non-test) code in Solr's "core" modules. SolrClient usage in tests and usage in other build modules (e.g. contribs, solrj, etc.) still need converted.

Tests

Existing tests continue to pass. New unit tests added in URLUtilsTest.

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.
  • I have added tests for my changes.

StreamingSolrClients now creates its internal clients using node base
URLs (e.g. "host:8983/solr") and `withDefaultCollection`.
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 production (i.e. non-test) code in the 'core'
module to use the `withDefaultCollection` builder method in lieu of
using core-based URLs directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should SolrCLI.java's "normalizeSolrUrl" method be moved over to here, or reuse some of these methods???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that'd probably make sense (moving normalizeSolrUrl that is). We've got this nice class just for those sort of utilities, so might as well use it 🤷

The only hiccup I'd imagine that might cause is that it looks like nSU uses a static 'CLIO' object that SolrCLI has access to. But I'm sure there's ways to straighten that out...

@gerlowskija gerlowskija merged commit 8e9c513 into apache:main Jan 14, 2024
4 checks passed
gerlowskija added a commit that referenced this pull request Jan 14, 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 production (i.e. non-test) code in the 'core'
module to use the `withDefaultCollection` builder method in lieu of
using core-based URLs directly.
@gerlowskija gerlowskija deleted the SOLR-17066-switch-clients-off-coreurl branch January 16, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants