Skip to content

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 18, 2025
Merged

Fix ES|QL build.gradle for configuration-cache #125097

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a62aaa7
Fix build.gradle and improved some generating code
craigtaverner Mar 18, 2025
d7db327
Merge branch 'main' into fix_esql_gradle
craigtaverner Mar 18, 2025
cbf60f8
Update x-pack/plugin/esql/build.gradle
craigtaverner Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions docs/reference/query-languages/esql/_snippets/operators/types/not_like.md
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions docs/reference/query-languages/esql/_snippets/operators/types/not_rlike.md
View file
Open in desktop

This file was deleted.

46 changes: 31 additions & 15 deletions x-pack/plugin/esql/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,18 @@ tasks.named("test").configure {
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 {
Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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.

include "**/types/*.md" // Recursively include all types/*.md files (effectively counting functions and operators)
}
def imagesTree = fileTree(imagesFolder).matching {
include "**/*.svg" // Recursively include all SVG files
}
def kibanaTree = fileTree(kibanaFolder).matching {
include "**/*.json" // Recursively include all JSON files
}

doLast {
List snippets = fileTree(snippetsFolder).matching {
include "**/types/*.md" // Recursively include all types/*.md files (effectively counting functions and operators)
}.files.collect { it.name }
def snippets = snippetsTree.files.collect { it.name }
int countSnippets = snippets.size()
if (countSnippets == 0) {
logger.quiet("ESQL Docs: No function/operator snippets created. Skipping sync.")
Expand All @@ -113,19 +120,27 @@ tasks.named("test").configure {
from snippetsFolder
into snippetsDocFolder
include '**/*.md'
preserve {
// The snippets directory contains generated and static content, so we must preserve all MD files.
include '**/*.md'
if (countSnippets <= 100) {
Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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.

Copy link
Contributor

@idegtiarenko idegtiarenko Mar 18, 2025

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?

Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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'

Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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.

Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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.

// If we do not run the full test of tests, do not attempt to remove potentially unused files
preserve {
// The snippets directory contains generated and static content, so we must preserve all MD files.
include '**/*.md'
}
} else {
// If we do not run the full test of tests, be careful about what we need to preserve
preserve {
// The lists are static, and the operators are a mix of generated and static content
include '*.md', '**/operators/*.md', '**/operators/**/*.md', '**/lists/*.md'
}
}
}
}

List images = fileTree(imagesFolder).matching {
include "**/*.svg" // Recursively include all SVG files
}.files.collect { it.name }
List images = imagesTree.files.collect { it.name }
int countImages = images.size()
Closure replaceFont = line -> {
// The es-docs team has a recommended set of fonts for use with code, and they size similarly to the previous Roboto Mono, which is no longer available in the docs webpage
// We do not change the original SVG generator to use these because it requires the fonts to exist in the JVM running the code
line.replaceAll(
/font-family:\s*Roboto Mono[^;]*;/,
'font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;'
Expand All @@ -139,17 +154,18 @@ tasks.named("test").configure {
from imagesFolder
into imagesDocFolder
include '**/*.svg'
preserve {
// Some operator files are currently static, so we must preserve them all
include '**/*.svg'
if (countImages <= 100) {
// If we do not run the full test of tests, do not attempt to remove potentially unused files
preserve {
// Some operator files are currently static, so we must preserve them all
include '**/*.svg'
}
}
filter replaceFont
}
}

List kibana = fileTree(kibanaFolder).matching {
include "**/*.json" // Recursively include all JSON files
}.files.collect { it.name }
List kibana = kibanaTree.files.collect { it.name }
int countKibana = kibana.size()
if (countKibana == 0) {
logger.quiet("ESQL Docs: No function/operator kibana docs created. Skipping sync.")
Expand Down
2 changes: 1 addition & 1 deletion ...in/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/DocsV3Support.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ private String getLink(String key) {
// Various other remaining old asciidoc links
// <<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");
Copy link
Contributor Author

@craigtaverner craigtaverner Mar 18, 2025

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.

case "search-aggregations-bucket-histogram-aggregation" -> makeLink(
key,
"",
Expand Down
Loading