Use project dependency instead of substitutions for distributions #37730
Conversation
Currently integration tests which use either bwc snapshot versions or the current version of elasticsearch depend on project substitutions to link to the build of those artifacts. Likewise, vagrant tests use dependency substitutions to get to bwc snapshots of rpm and debs. This commit changes those to depend on the relevant project/configuration and removes the dependency substitutions for distributions we do not publish.
|
Pinging @elastic/es-core-infra |
There was a problem hiding this comment.
This looks good to me, the only think I don't like about it is that we don't have a test to assure us we don't break it again for external plugin authors.
I'm wondering if given the complexity it adds is it worth downloading trough the download service, or would it be better to continue using our own maven repos.
If my understanding is correct that would not defeat your goals with this set of PRs.
This would help us keep the simplifying assumption that anything meant for external consumption is published as a maven artifact. That would mean we can fully utilize the simulated local repo in our tests to check that we correctly expose all artifacts meant for consumption and would allow us to avoid duplicating the logic to re-map the distributions to what was built locally - we could continue to use the substitutions as before.
Granted, tests would not download dependencies in the same way as users would, but we can cover that part as a check in the release process or a release test if we must that would only check that the artifacts are there in the download service and they match the ones in the maven repo.
I don't like this because it means adding artifacts back to maven which don't really belong in maven. That is about jars, but these are our distributions. |
This won't work when we add classifiers. I found those simply do not work with project substitutions. |
We do have a test, it is the example plugins tests you added! Those failing is what made me realize it was broken for plugin authors. |
Maybe I'm missing something, but it seems the way we pass in the distributions to those tests circumvented part of the test, and it didn't catch the failure we observed in es-hadoop. |
|
@elasticmachine run elasticsearch-ci/2 |
* elastic/master: Optimize warning header de-duplication (elastic#37725) Bubble exceptions up in ClusterApplierService (elastic#37729) SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK (elastic#37803) Remove unused ThreadBarrier class (elastic#37666) Add built-in user and role for code plugin (elastic#37030) Consolidate testclusters tests into a single project (elastic#37362) Fix docs for MappingUpdatedAction SQL: Introduce SQL DATE data type (elastic#37693) disabling bwc test while backporting elastic#37639 Mute ClusterDisruptionIT testAckedIndexing Set acking timeout to 0 on dynamic mapping update (elastic#31140) Remove index audit output type (elastic#37707) Mute FollowerFailOverIT testReadRequestsReturnsLatestMappingVersion [ML] Increase close job timeout and lower the max number (elastic#37770) Remove Custom Listeners from SnapshotsService (elastic#37629) Use m_m_nodes from Zen1 master for Zen2 bootstrap (elastic#37701) Fix index filtering in follow info api. (elastic#37752) Use project dependency instead of substitutions for distributions (elastic#37730) Update authenticate to allow unknown fields (elastic#37713) Deprecate HLRC EmptyResponse used by security (elastic#37540)
Currently integration tests which use either bwc snapshot versions or
the current version of elasticsearch depend on project substitutions to
link to the build of those artifacts. Likewise, vagrant tests use
dependency substitutions to get to bwc snapshots of rpm and debs.
This commit changes those to depend on the relevant project/configuration
and removes the dependency substitutions for distributions we do not
publish.