-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Update Documentation URLs #125089
base: main
Are you sure you want to change the base?
Update Documentation URLs #125089
Conversation
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.
Pull Request Overview
This PR updates deprecation documentation URLs to use consistent short URLs and a revised versioning scheme, simplifying backports and clarifying version relevance.
- Updated URLs in deprecation notices for ML, Index, ILM policy, Data Stream, and Node checks
- Adjusted the URL naming scheme to reflect when deprecations were introduced instead of the current major version
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/MlDeprecationChecker.java | Updated URLs for data feed query, aggregation, and ML model snapshot deprecations |
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecker.java | Replaced long upgrade URLs with short URLs for transform, index version, translog retention, shared data path, and simplefs settings |
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IlmPolicyDeprecationChecker.java | Updated URL for frozen indices deprecation |
x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java | Adjusted expected URLs to match new short URL scheme for index and transform deprecations |
x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java | Updated deprecation messages and URLs for data streams using dynamic version values |
x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java | Changed expected URLs for realm and watcher settings deprecations |
x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IlmPolicyDeprecationCheckerTests.java | Updated expected URL for frozen action deprecation in ILM policies |
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java | Revised URLs for shared data path, realm prefixes, EQL enabled, and watcher bulk settings |
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationChecker.java | Updated deprecation messages and URLs for data stream indices using dynamic version values |
Comments suppressed due to low confidence (4)
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/MlDeprecationChecker.java:100
- Verify that the new URL for ML model snapshot deprecation correctly reflects the intended documentation; ensure it aligns with the deprecation context.
"https://ela.st/es-deprecation-8-model-snapshot-version",
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecker.java:104
- Confirm that 'https://ela.st/es-deprecation-9-transform-destination-index' is the correct target URL for the transform destination index deprecation in every occurrence.
"https://ela.st/es-deprecation-9-transform-destination-index",
x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java:1021
- Ensure that the new URL for the watcher bulk concurrency setting ('https://ela.st/es-deprecation-8-watcher-settings') accurately points to the documentation specific to that deprecated setting.
String url = "https://ela.st/es-deprecation-8-watcher-settings";
x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java:60
- Review the concatenation of Version.CURRENT.major with '0.0' in the deprecation message to ensure it produces the intended version format.
"Old data stream with a compatibility version < " + Version.CURRENT.major + "0.0",
Also updates the text to use a dynamic placeholder for version number so code can be backported without change
2866608
to
848c260
Compare
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.
LGTM to me in general but I do have one question about how we treat index compatibility.
...recation/src/main/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationChecker.java
Outdated
Show resolved
Hide resolved
"Old data stream with a compatibility version < " + Version.CURRENT.major + "0.0 Has Been Ignored", | ||
"https://ela.st/es-deprecation-ds-reindex", | ||
"This data stream has read only backing indices that were created before Elasticsearch " | ||
+ Version.CURRENT.major | ||
+ "0.0 and have been marked as OK to remain read-only after upgrade", |
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.
Nit: When we check the lucene compatibility for indices we hardcode version 9.0.0
but when we check it for data streams it's not version specific. I am wondering why we have different approaches in these two cases; I initially expected them to be treated in the same way.
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.
Yeah it's mostly a choice on my part... We own the DS check so I felt comfortable improving it but not the index one so I didn't want to go messing with the logic there too much, especially as I'm not sure what Kibana does with those messages and if it tries to parse them.
Might be being a little over cautious so happy to go back and give the index check the same treatment or, to roll this one back so it's hard coded the same as the main index check.
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 see, I think we own both so if this is stopping you then I understand. I think Kibana should be ok, I do not think it parses the messages, if you have time we could double check it with them; otherwise, we can leave it as it is.
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.
Ahhh good stuff. I thought core owned the index deprecation notices so avoided it... Anyway I have updated those to now use a dynamic version number but was careful to make sure the messages didn't actually change their content to sidestep any kibana issues.
...ion/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java
Outdated
Show resolved
Hide resolved
…k/deprecation/DataStreamDeprecationChecker.java Co-authored-by: Mary Gouseti <[email protected]>
…k/deprecation/DataStreamDeprecationCheckerTests.java Co-authored-by: Mary Gouseti <[email protected]>
This PR moves the majority of our deprecation notices to using short URLs and updates the URL naming scheme to be more consistent.
Instead of using the current major version number in the URL, this PR changes things so we instead use the version number the deprecation was introduced in. This rescues backporting workload and makes it clearer which URLs remain relevant.
Some URLs are placeholders at the moment while we await the new 9.0 documentation.
Here's the table of links:
Fixes ES-9508