xvrl commented on PR #15106: URL: https://github.com/apache/druid/pull/15106#issuecomment-1760364872
> could you please reference a failed build which have failed because of that caching? I have experienced this locally where I had artifacts from a different build causing integration tests to fail. This change removes that possibility. See also further below in my comment for an example that happened as part of this change. > since the cache is labeled by the pom hashes I don't think its showstopper to have installed artifacts from the project being built The pom hash is not sufficient, different code could have the same pom hash, causing incorrect artifacts in the maven repository from being used at runtime. Some tests may also accidentally pass due to artifacts being present instead of failing if they are not rebuilt. Only those cases where we explicitly want to reuse artifacts from a previous build step for same commit should be restoring maven install artifacts. Those are already marked as such https://github.com/apache/druid/blob/master/.github/workflows/unit-and-integration-tests-unified.yml#L72-L80 > my problem with using random maven commands to download artifacts is that they don't always do everything - for example: druid has some extra maven plugin stuff which downloads a nodejs.tgz into the local m2 repo fair, although the goal of caching is to download as many dependencies as possible without affecting the integrity of the build. If we miss a few it should not be a dealbreaker. I'm trying to achieve this with as few commands a possible. We may need to resolve additional dependencies explicitly if they are only called during some specific phases of the build. It looks like `mvn dependency:resolve` trips up trying to download druid artifacts in submodules so we may need a different approach than what I tried regardless. This is actually a good example of where it worked locally because my maven repo had druid artifacts from a previous mvn install, but failed in CI without those artifacts. Our default maven cache should only ever contain artifacts already in maven central. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
