[FIXED] -- handle trailing slash in Solr index URL for core reload. by DhavalGojiya · Pull Request #1968 · django-haystack/django-haystack · GitHub
Skip to content

[FIXED] -- handle trailing slash in Solr index URL for core reload.#1968

Merged
cclauss merged 2 commits into
django-haystack:masterfrom
DhavalGojiya:bugfix/solr-core-name-extraction
Jul 10, 2025
Merged

[FIXED] -- handle trailing slash in Solr index URL for core reload.#1968
cclauss merged 2 commits into
django-haystack:masterfrom
DhavalGojiya:bugfix/solr-core-name-extraction

Conversation

@DhavalGojiya

@DhavalGojiya DhavalGojiya commented May 18, 2024

Copy link
Copy Markdown
Contributor
  1. When running python manage.py build_solr_schema --reload-core=True, it is crucial to correctly extract the Solr core name from the URL defined in the settings.
  2. The existing implementation fails if the URL ends with a trailing slash, resulting in an empty core name due to the final slash being considered as a separator.
  3. Previous Implementation: core = settings.HAYSTACK_CONNECTIONS[using]["URL"].rsplit("/", 1)[-1]
    This splits the URL by the last / and takes the last part as the core name. If the URL ends with a slash, the result is an empty string and unable to reload core error will raised.
  4. The rstrip("/") method removes any trailing slashes from the URL, ensuring the core name is correctly extracted even if the URL ends with a slash.

@yatishTrootech

Copy link
Copy Markdown

@cclauss

cclauss commented May 21, 2024

Copy link
Copy Markdown
Contributor

Is there any way to create a test that fails on the current code and proves that this change fixes that failure?

@DhavalGojiya

DhavalGojiya commented May 21, 2024

Copy link
Copy Markdown
Contributor Author

Is there any way to create a test that fails on the current code and proves that this change fixes that failure?

To reproduce the issue, update settings.py in your Django project with the following configuration:

HAYSTACK_CONNECTIONS = {
    'default': {
        'ENGINE': 'haystack.backends.solr_backend.SolrEngine',
        'URL': 'http://localhost:9001/solr/my_core/',
        'TIMEOUT': 60 * 5,
        'INCLUDE_SPELLING': True,
        'BATCH_SIZE': 100,
        'EXCLUDED_INDEXES': ['thirdpartyapp.search_indexes.BarIndex']
    }
}

Screenshot from 2024-05-21 11-27-51-q1nposhcgt8efgp6der33b4rjy

Running the reload-core management command will fail because it cannot parse "my_core" from the URL due to the trailing slash.
Note: Slash at the end or not doesn't affect any Solr API, but Django haystack Python code depends on it and it's very common for developers to add the slash at the end of the url.

To fix this, remove the trailing slash from the URL:

'URL': 'http://localhost:9001/solr/my_core'

My PR corrects this issue by handling URLs with or without a trailing slash.

Attach: Image attached for the failure of this management command when there is slash at the end in URL
CC : @yatishTrootech | @cclauss | @acdha

@yatishTrootech

Copy link
Copy Markdown

Is there any way to create a test that fails on the current code and proves that this change fixes that failure?

To reproduce the issue, update settings.py in your Django project with the following configuration:

HAYSTACK_CONNECTIONS = {
    'default': {
        'ENGINE': 'haystack.backends.solr_backend.SolrEngine',
        'URL': 'http://localhost:9001/solr/my_core/',
        'TIMEOUT': 60 * 5,
        'INCLUDE_SPELLING': True,
        'BATCH_SIZE': 100,
        'EXCLUDED_INDEXES': ['thirdpartyapp.search_indexes.BarIndex']
    }
}

Screenshot from 2024-05-21 11-27-51-q1nposhcgt8efgp6der33b4rjy

Running the reload-core management command will fail because it cannot parse "my_core" from the URL due to the trailing slash. Note: Slash at the end or not doesn't affect any Solr API, but Django haystack Python code depends on it and it's very common for developers to add the slash at the end of the url.

To fix this, remove the trailing slash from the URL:

'URL': 'http://localhost:9001/solr/my_core'

My PR corrects this issue by handling URLs with or without a trailing slash.

Attach: Image attached for the failure of this management command when there is slash at the end in URL CC : @yatishTrootech | @cclauss

PS: I will try to develop the test case for what you are talking about very soon.

Or it will be better if you record a video of test case and upload it here

@claudep claudep left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! However test_haystack/solr_tests/test_solr_management_commands.py should be completed with a regression test for this fix.

@DhavalGojiya DhavalGojiya force-pushed the bugfix/solr-core-name-extraction branch from ea75a80 to f050d86 Compare June 22, 2024 18:40
@DhavalGojiya

Copy link
Copy Markdown
Contributor Author

Thanks! However test_haystack/solr_tests/test_solr_management_commands.py should be completed with a regression test for this fix.

Thank you for your response. I have rebased from the master branch and pushed again, so my previous commit is no longer present. I will try to add regression test case for this very soon.

- When running `python manage.py build_solr_schema --reload_core=True`, it is crucial to correctly extract the Solr core name from the URL defined in the settings.
- The existing implementation failed if the URL ended with a trailing slash, resulting in an empty core name due to the final slash being considered as a separator.

Added test cases:
- `test_build_solr_schema_reload_core_with_trailing_slash`
- `test_build_solr_schema_reload_core_without_trailing_slash`

These ensure that the core reload logic works correctly regardless of whether the Solr URL has a trailing slash.
@DhavalGojiya DhavalGojiya force-pushed the bugfix/solr-core-name-extraction branch from 34ea372 to f0ff2c4 Compare February 23, 2025 17:11
@DhavalGojiya

Copy link
Copy Markdown
Contributor Author

@cclauss | @claudep
The requested test cases have been added to test_haystack/solr_tests/test_solr_management_commands.py.

Added test cases:

  • test_build_solr_schema_reload_core_with_trailing_slash
  • test_build_solr_schema_reload_core_without_trailing_slash

Test Results:

haystack_test_case

@yatishTrootech

Copy link
Copy Markdown

@cclauss | @claudep The requested test cases have been added to test_haystack/solr_tests/test_solr_management_commands.py.

Added test cases:

  • test_build_solr_schema_reload_core_with_trailing_slash
  • test_build_solr_schema_reload_core_without_trailing_slash

Test Results:

haystack_test_case

Seems good to me @DhavalGojiya

@abhishek145607

Copy link
Copy Markdown

@cclauss cclauss requested a review from yatishTrootech July 10, 2025 14:58
@cclauss cclauss enabled auto-merge (squash) July 10, 2025 14:58
@cclauss cclauss dismissed claudep’s stale review July 10, 2025 17:05

Tests included and pass.

@cclauss cclauss merged commit 8330862 into django-haystack:master Jul 10, 2025
18 checks passed
@DhavalGojiya DhavalGojiya deleted the bugfix/solr-core-name-extraction branch October 26, 2025 18:43
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.

6 participants