-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Use lowercase letters for zstd, zstdnodict compression codecs. #7231
Conversation
Signed-off-by: Mulugeta Mammo <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
* lowercase letters when registering the codec so that we remain consistent with | ||
* the other compression codecs: default, lucene_default, and best_compression. | ||
* | ||
* @param mode The compression codec (ZSTD or ZSTDNODICT). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be lowercase in the @param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter is a Java enum.
public Lucene95CustomCodec(Mode mode, int compressionLevel) { | ||
super(mode.name(), new Lucene95Codec()); | ||
super(mode.name().toLowerCase(Locale.ROOT), new Lucene95Codec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we still want toLowerCase
it here? If we want these to be case-sensitive we should get rid of anything that says ZSTD
and replace it with zstd
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is to simplify a bit Mode
conventions: since this is Java's enum, the convention is to use uppercased Java members (mode.name()
)
Signed-off-by: Mulugeta Mammo <[email protected]> (cherry picked from commit e791d46) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
#7260) (cherry picked from commit e791d46) Signed-off-by: Mulugeta Mammo <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…earch-project#7231) Signed-off-by: Mulugeta Mammo <[email protected]>
…earch-project#7231) Signed-off-by: Mulugeta Mammo <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
OpenSearch supports:
default
,lucene_default
,best_compression
, andZSTD
compression codecs. This PR changesZSTD
tozstd
to maintain consistency in naming compression codecs.Issues Resolved
A follow up to PR: #7171.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.