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

[7.16] Drop non-setting deprecations down to warning level #80374

Merged
merged 14 commits into from
Nov 10, 2021

Conversation

jakelandis
Copy link
Contributor

@jakelandis jakelandis commented Nov 4, 2021

Complementary to #79665 this change
will reduce the deprecation logging level to WARN for (most) all non-settings that
continue to exist in 8.0+.

===

Note - I identified which logs needed to be reduced from the work at #80167. In that PR I used IntelliJ to refactor ALL deprecations in 8.0+ to log at warning level. One of those commits 967264e (#80167) that got squashed in the final merge was only for non-settings. So that commit has all of the non-settings that still exist in 8.0, so I cherry-picked that commit back to 7.16 and only merged files that merged cleanly. So there are likely a couple critical level that could be adjusted since they had additional logic that would require additional manual evaluation. I also had to update the tests since in 7.16 the default assertion is for critical, but in 8.0+ the default assertion is for warning level. There were only a few outliers and have confidence that the list here is accurate and mostly complete.

@jakelandis jakelandis added >non-issue :Core/Infra/Settings Settings infrastructure and APIs labels Nov 5, 2021
@jakelandis jakelandis marked this pull request as ready for review November 5, 2021 15:30
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Nov 5, 2021
@elasticmachine
Copy link
Collaborator

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

@jakelandis jakelandis added v7.16.0 and removed v7.16.0 labels Nov 5, 2021
@jakelandis
Copy link
Contributor Author

jakelandis commented Nov 5, 2021

After this change, there will be

189 usages of org.elasticsearch.common.logging.DeprecationLogger#critical
and
82 uses of org.elasticsearch.common.logging.DeprecationLogger#warn

in the 7.16 branch.

Copy link
Member

@dakrone dakrone 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 left one question

@@ -511,7 +511,7 @@ public void testRewriteForUpdate() throws IOException {
assertTrue(transformConfigRewritten.getSettings().getDatesAsEpochMillis());
assertFalse(transformConfigRewritten.getSettings().getAlignCheckpoints());

assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);
assertWarnings(org.apache.logging.log4j.Level.WARN, TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason these are fully qualified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird .. Intellij chose to do that. Fixed in 0718f87 (#80374)

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM

@jakelandis
Copy link
Contributor Author

@elasticmachine update branch

@jakelandis jakelandis merged commit 6358afb into elastic:7.16 Nov 10, 2021
@jakelandis jakelandis deleted the reduce_deprecations_7_16 branch November 10, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Settings Settings infrastructure and APIs >non-issue Team:Core/Infra Meta label for core/infra team v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants