Improve testcluster distribution artifact handling #38933
Conversation
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
mark-vieira
added
>non-issue
:Delivery/Build
|
Pinging @elastic/es-core-infra |
|
@atorok I'm not sure what all version labels are appropriate here. Please add what is necessary and I'll backport as needed. |
|
Additionally, https://scans.gradle.com/s/p5bai45p5ycho/tests/orjfcz7hu5lqq-eajrubpg6r7za I had to do some hacking of the way we build the distribution dependency string to get it to work. Perhaps this is still WIP? |
|
Unfortunately the tests broke while they were muted. |
|
Back-port wise I find that taking the build changes all the way back makes life easier. The back-ports ( at least to changes in buildSrc ) should be straight forward since I'we been following this policy for a while. |
| @@ -98,7 +116,15 @@ public void apply(Project project) { | |||
| // the clusters will look for artifacts there based on the naming conventions. | |||
| // Tasks that use a cluster will add this as a dependency automatically so it's guaranteed to run early in | |||
| // the build. | |||
| rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, SyncTestClustersConfiguration.class); | |||
| rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, Sync.class, sync -> { | |||
| sync.from((Callable<List<FileTree>>) () -> | |||
There was a problem hiding this comment.
wondering why the cast is needed here
There was a problem hiding this comment.
CopySpec.from() simply takes Object... as an argument. Each item passed can be any of the types listed here. We can't just pass a raw lambda, as Java has now idea what SAM type to cast it to. So we have to explicitly cast it as one of the supported types. We don't have this problem in Groovy as { } returns an instance of Closure.
There's actually no real need to include the generic type arguments here, Gradle will figure this out at runtime, but its more explicit and folks reading the code immediately see what that lambda returns.
There was a problem hiding this comment.
Additionally, if it's not obvious, we wrap all this in a Callable because we are iterating over a Configuration here so we can return a FileTree from each resolved artifact. Doing so causes the Configuration to be resolved (and files to be downloaded) so we want to defer this until task execution. Or in this case more specifically, during task graph calculation.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
* master: Address some CCR REST test case flakiness (elastic#38975) Edits to text in Completion Suggester doc (elastic#38980) SQL: doc polishing [DOCS] Fixes broken formatting SQL: Polish the rest chapter (elastic#38971) Remove `nGram` and `edgeNGram` token filter names (elastic#38911) Add an exception throw if waiting on transport port file fails (elastic#37574) Improve testcluster distribution artifact handling (elastic#38933) Advance max_seq_no before add operation to Lucene (elastic#38879) Reduce global checkpoint sync interval in disruption tests (elastic#38931) [test] disable packaging tests for suse boxes Relax testStressMaybeFlushOrRollTranslogGeneration (elastic#38918) [DOCS] Edits warning in put watch API (elastic#38582) Fix serialization bug in ShardFollowTask after cutting this class over to extend from ImmutableFollowParameters. [DOCS] Updates methods for upgrading machine learning (elastic#38876)
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether.
Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault.
An example of the error message looks like this: