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-17575: Fixed broken backwards compatibility with the legacy "langid.whitelist" config in Solr Langid #2886

Merged

Conversation

azagniotov
Copy link
Contributor

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

Description

Making sure that backwards compatibility with the legacy "langid.whitelist" config in Solr Langid works as expected. In other words, the language detection runtime behavior should be the same when the ISO 639-1 language codes specified in the legacy "langid.whitelist" OR the current (default) "langid.allowlist".

Solution

Making sure that if the legacy "langid.whitelist" is configured, its value is used as the default value when reading the LANG_ALLOWLIST parameter.

Tests

  • Added a sanity unit test to demonstrate that the behavior using the current (default) "langid.allowlist" is as expected.
  • Added a unit test to demonstrate that the backwards compatibility with the legacy "langid.whitelist" works as expected.

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.
  • (N/A) I have added documentation for the Reference Guide

Copy link
Contributor

@janhoy janhoy 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 the quick turnaround. I enabled PR test runs and see that a the new tests fail

   >     org.junit.ComparisonFailure: expected:<[no]> but was:<[sv]>

See my comments on how to greatly simplify the tests :)

@epugh
Copy link
Contributor

epugh commented Nov 27, 2024

glad to see we are tackling fixing a bug. In 10x we should move forward and delete the langid.whitelist.....

@janhoy
Copy link
Contributor

janhoy commented Nov 27, 2024

glad to see we are tackling fixing a bug. In 10x we should move forward and delete the langid.whitelist.....

For sure. I expect there to be lots of such cleanup commits on the main branch in the coming weeks.

@janhoy
Copy link
Contributor

janhoy commented Nov 28, 2024

Good. Only one thing missing now. Add a CHANGES.txt line under 9.8.0 Bugfix section, with your own and my name. I’ll then merge and backport.

@azagniotov
Copy link
Contributor Author

janhoy

@janhoy done!

@janhoy janhoy merged commit cebdb2d into apache:main Nov 28, 2024
1 check passed
janhoy pushed a commit that referenced this pull request Nov 28, 2024
…gid.whitelist" config in Solr Langid (#2886)

Co-authored-by: Alexander Zagniotov <azagniotov@box.com>
Co-authored-by: Jan Høydahl <janhoy@apache.org>
(cherry picked from commit cebdb2d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants