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

Searchable snapshots GA #504

Merged

Conversation

rhysxevans
Copy link
Contributor

This follows the announcement here https://github.com/opensearch-project/opensearch-build/blob/main/release-notes/opensearch-release-notes-2.7.0.md This means the experimental flag only needs to be added if version is less than 2.7.0

This follows the announcement here https://github.com/opensearch-project/opensearch-build/blob/main/release-notes/opensearch-release-notes-2.7.0.md
This means the experimental flag only needs to be added if version is less than 2.7.0

Signed-off-by: Rhys Evans <[email protected]>
Copy link
Collaborator

@idanl21 idanl21 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 improvement

@idanl21
Copy link
Collaborator

idanl21 commented May 4, 2023

@rhysxevans
Just take a look why the tests are failing

idanl21
idanl21 previously approved these changes May 10, 2023
Comment on lines 254 to 257
if err == nil && ver1.LessThan(ver2) {
lessThan = true
}
return lessThan
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be written shorter:

Suggested change
if err == nil && ver1.LessThan(ver2) {
lessThan = true
}
return lessThan
return err == nil && ver1.LessThan(ver2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @swoehrl-mw, I have made the changes requested, please let me know if there is anything else

opensearch-operator/pkg/builders/cluster.go Outdated Show resolved Hide resolved
Signed-off-by: Rhys Evans <[email protected]>
@swoehrl-mw swoehrl-mw merged commit 41ee305 into opensearch-project:main May 12, 2023
@prudhvigodithi
Copy link
Member

prudhvigodithi commented May 26, 2023

This feature is GA In 2.7 https://opensearch.org/blog/searchable-snapshots/, should we consider removing the experimental flag?
@segalziv @idanl21 @swoehrl-mw @rhysxevans

@swoehrl-mw
Copy link
Collaborator

@prudhvigodithi

This feature is GA In 2.7 https://opensearch.org/blog/searchable-snapshots/, should we consider removing the experimental flag?

This PR already dealt with not using the feature flag for 2.7+. And as long as the operator also supports older opensearch versions we will need to keep the logic to add the feature flag.

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.

4 participants