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

declare deprecation of enterprise search plugin #22

Merged
merged 10 commits into from
Jan 3, 2025

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Dec 19, 2024

The Enterprise Search product is deprecated and will not be included in the Elastic Stack 9.x release.
As a result, the App Search and Workplace Search plugins are also deprecated. These plugins will only receive security updates and critical bug fixes moving forward.

This commit added deprecation log

fixes: #20, #21

log_message = "The App Search product is deprecated and excluded from the version 9 of the Elastic Stack. " +
"This plugin is deprecated and will only receive security updates and critical bug fixes. " +
"We recommend transitioning to our native Elasticsearch tools. " +
"For more details, please visit https://www.elastic.co/guide/en/enterprise-search/current/app-search-workplace-search.html"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flexitrev same question here about the link https://www.elastic.co/guide/en/enterprise-search/current/app-search-workplace-search.html
The instruction is a bit vague to me

Choose a reason for hiding this comment

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

I changed the link to the Elasticsearch connector. I think it's better to direct customers to the software they should use rather than another deprecation message.

"This plugin is deprecated and will only receive security updates and critical bug fixes. " +
"We recommend transitioning to our native Elasticsearch tools. " +
"For more details, please visit https://www.elastic.co/guide/en/enterprise-search/current/app-search-workplace-search.html"
@deprecation_logger.deprecated log_message
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@deprecation_logger.deprecated log_message
deprecation_logger.deprecated log_message

Is using an instance variable here an old pattern? https://github.com/logstash-plugins/logstash-mixin-deprecation_logger_support/blob/main/lib/logstash/plugin_mixins/deprecation_logger_support/legacy_init_adapter.rb

"This plugin is deprecated and will only receive security updates and critical bug fixes. " +
"We recommend transitioning to our native Elasticsearch tools. " +
"For more details, please visit https://www.elastic.co/guide/en/enterprise-search/current/app-search-workplace-search.html"
@deprecation_logger.deprecated log_message
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@deprecation_logger.deprecated log_message
deprecation_logger.deprecated log_message

Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

LGTM. I think the 3.x series of this plugin will be going in to 9.x for LS and 2.x -> 8.x. Do we need the deprecation log in the 2 series as well? Or does this only apply to removal in the 9.x?

@kaisecheng
Copy link
Contributor Author

@donoghuc Thanks for the reviews. We maintain a single series of plugin. 3.0.0 is included in 8.17.
Although the default distribution of v9 will not include this plugin, user can manually install it. Whenever, the plugin is used, the deprecation log should print and give instructions to the next steps.

@donoghuc
Copy link
Contributor

Ah, sorry I confused this with some of the other plugins i've been working on. Thanks that makes sense! 🙃 ERRTOOMANYPLUGINS

Copy link

@flexitrev flexitrev left a comment

Choose a reason for hiding this comment

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

I made some changes to the deprecation message that I think are a bit clearer.

lib/logstash/outputs/elastic_app_search.rb Outdated Show resolved Hide resolved
lib/logstash/outputs/elastic_app_search.rb Outdated Show resolved Hide resolved
lib/logstash/outputs/elastic_app_search.rb Outdated Show resolved Hide resolved
log_message = "The App Search product is deprecated and excluded from the version 9 of the Elastic Stack. " +
"This plugin is deprecated and will only receive security updates and critical bug fixes. " +
"We recommend transitioning to our native Elasticsearch tools. " +
"For more details, please visit https://www.elastic.co/guide/en/enterprise-search/current/app-search-workplace-search.html"

Choose a reason for hiding this comment

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

I changed the link to the Elasticsearch connector. I think it's better to direct customers to the software they should use rather than another deprecation message.

@kaisecheng kaisecheng requested a review from flexitrev January 3, 2025 14:37
Copy link

@flexitrev flexitrev left a comment

Choose a reason for hiding this comment

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

LGTM

@kaisecheng kaisecheng merged commit 864cb2b into logstash-plugins:main Jan 3, 2025
2 checks passed
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.

Add deprecation warnings to the usage of this plugin
4 participants