-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix ES|QL build.gradle for configuration-cache #125097
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
Conversation
|
Pinging @elastic/es-docs (Team:Docs) |
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| def imagesDocFolder = file("${rootDir}/docs/reference/query-languages/esql/images") | ||
| def snippetsDocFolder = file("${rootDir}/docs/reference/query-languages/esql/_snippets") | ||
| def kibanaDocFolder = file("${rootDir}/docs/reference/query-languages/esql/kibana") | ||
| def snippetsTree = fileTree(snippetsFolder).matching { |
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.
The fix for --configuration-cache is essentially to move these calls to fileTree out of the doLast while still making sure they run after the tests are finished.
| preserve { | ||
| // The snippets directory contains generated and static content, so we must preserve all MD files. | ||
| include '**/*.md' | ||
| if (countSnippets <= 100) { |
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.
We used to delete all files if we ran more than 1 test, which is extreme. This way we only delete all files if we probably run the entire suite.
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 am not following this. Both branches start with If we do not run the full test of tests comment.
How does it know what files could be preserved when running individual tests?
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 agree the comment is ambiguous. The second half of the comment is better. Anyway, the way it works is if we run only a subset of the tests (usually just one, hence the old rule), then it uses include '**/*.md' to keep all old files. If, instead, we run all the tests, it will try to delete unused files. We used to delete all unused files (too many), but now we delete only the ones we feel more confident are actually generated. The more complex line to handle this is:
include '*.md', '**/operators/*.md', '**/operators/**/*.md', '**/lists/*.md'
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 verified this line by running all tests and seeing what got deleted, and verifying that it was nothing we didn't already want to delete.
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 deleted the not in the second comment below, since it was the source of the confusion.
| // <<match-field-params,match query parameters>> | ||
| return switch (parts[0]) { | ||
| case "match-field-params" -> makeLink(key, "", "/reference/query-languages/query-dsl-match-query.md"); | ||
| case "match-field-params" -> makeLink(key, "", "/reference/query-languages/query-dsl/query-dsl-match-query.md"); |
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.
This is a loose end from the docs restructuring PR that merged yesterday. It should have been in that PR.
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! Minor thing around some comments Ievgen commented, which may probably be rewritten
Earlier work on the ES|QL port of docs to V3 introduced an issue in the build.gradle file making it fail with --configuration-cache. This fixes that, as well as one other broken link and removes some unused files. In addition we bring back partial support for deleting unused files. It is tricky to have full support for this due to the mix of static and generated content, particularly in the operators snippets.
Earlier work on the ES|QL port of docs to V3 introduced an issue in the build.gradle file making it fail with --configuration-cache. This fixes that, as well as one other broken link and removes some unused files. In addition we bring back partial support for deleting unused files. It is tricky to have full support for this due to the mix of static and generated content, particularly in the operators snippets.
Earlier work on the ES|QL port of docs to V3 introduced an issue in the build.gradle file making it fail with
--configuration-cache. This fixes that, as well as one other broken link and removes some unused files.In addition we bring back partial support for deleting unused files. It is tricky to have full support for this due to the mix of static and generated content, particularly in the operators snippets.