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

Improve transient settings deprecation message #79504

Merged

Conversation

grcevski
Copy link
Contributor

Add the supplied transient settings names as part of the deprecation response, to help people understand which settings they need to convert to persistent settings.

Add the setting name as part of the deprecation response.
@grcevski grcevski added :Core/Infra/Core Core issues without another label v8.0.0 Team:Core/Infra Meta label for core/infra team auto-backport Automatically create backport pull requests when merged v7.16.0 labels Oct 19, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@masseyke masseyke requested a review from debadair October 19, 2021 19:05
@@ -10,13 +10,18 @@
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;

import java.util.Locale;
import java.util.stream.Collectors;

public class ClusterDeprecationChecks {
static DeprecationIssue checkTransientSettingsExistence(ClusterState state) {
if (state.metadata().transientSettings().isEmpty() == false) {
return new DeprecationIssue(DeprecationIssue.Level.WARNING,
"Transient cluster settings are in the process of being removed.",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like @debadair from our docs team has weighed in on this, and has recommended the message be "Transient cluster settings are deprecated" and the details be "Use persistent settings to configure your cluster." I like the idea of actually putting the setting names in the details like this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, I'll change the PR to use that exact wording :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @masseyke! I updated the wording to match the docs team recommendation.

Copy link
Member

@masseyke masseyke left a comment

Choose a reason for hiding this comment

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

Looks good to me

@grcevski grcevski merged commit 008c55b into elastic:master Oct 19, 2021
grcevski added a commit to grcevski/elasticsearch that referenced this pull request Oct 19, 2021
Update message as per docs team's suggestion.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.x

grcevski added a commit that referenced this pull request Oct 20, 2021
Update message as per docs team's suggestion.
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 20, 2021
* upstream/master: (24 commits)
  Implement framework for migrating system indices (elastic#78951)
  Improve transient settings deprecation message (elastic#79504)
  Remove getValue and getValues from Field (elastic#79516)
  Store Template's mappings as bytes for disk serialization (elastic#78746)
  [ML] Add queue_capacity setting to start deployment API (elastic#79433)
  [ML] muting rest compat test issue elastic#79518 (elastic#79519)
  Avoid redundant available indices check (elastic#76540)
  Re-enable BWC tests
  TEST Ensure password 14 chars length on Kerberos FIPS tests (elastic#79496)
  [DOCS] Temporarily remove APM links (elastic#79411)
  Fix CCSDuelIT for skipped shards (elastic#79490)
  Add other time accounting in HotThreads (elastic#79392)
  Add deprecation info API entries for deprecated monitoring settings (elastic#78799)
  Add note in breaking changes for nameid_format (elastic#77785)
  Use 'migration' instead of 'upgrade' in GET system feature migration status responses (elastic#79302)
  Upgrade lucene version 8b68bf60c98 (elastic#79461)
  Use Strings#EMPTY_ARRAY (elastic#79452)
  Quicker shared cache file preallocation (elastic#79447)
  [ML] Removing some code that's obsolete for 8.0 (elastic#79444)
  Ensure indexing_data CCR requests are compressed (elastic#79413)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v7.16.0 v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants