search

apache / incubator-kie-kogito-runtimes

Kogito Runtimes - Kogito is a cloud-native business automation technology for building cloud-ready business applications.
http://kogito.kie.org
Apache License 2.0
542 stars 209 forks source link

Avoid using m2 and accessing maven central overriding the settings #3599

Closed elguardian closed 3 months ago

elguardian commented 3 months ago

This is about fixing some order.

elguardian commented 3 months ago

@gitgabrio your comment is unrelated to the issues this is trying to fix and guard. So far you haven't offered better alternative to tackle both at the same time

gitgabrio commented 3 months ago

@elguardian Sorry, but this is the only motivation for this PR "This is about fixing some order." and, TBH, does not seems clear at all.

About the fix itself, we have other mvn invoker tests, where this is not needed at all, so it is not clear why here, and only here, this should be needed. In general term, IMO it only add another layer of confusion, because at a given point we would have the mvn invoker that uses artifacts stored in its own repo, and it would not be clear which one those are. Getting deeper, lets consider two kind of dependencies: artifacts that are part of kogito-runtimes (A), and those that are not (B).

  1. for the former (A), we must be sure that the tests (mvn invoker) always uses the ones that have been built by the PR, but with your proposal, we introduce the risk that the ones used by the invoker are different then the ones built in PR; if, on the other side, the IT tests uses some "final" version of the kogito-runtimes artifacts, then there is no reason to store them in a different repository

  2. about the latter (B), I do not see any reason to store them in a different repository instead of the "normal" one.

If there is some "caching" mechanism that, somehow, interfer with that (i.e., for some reason mvn invoker pickup kogito-runtimes dependencies that have not been built inside the current PR), then there is a misconfiguration of that caching mechanism; and, again, this fix instead of solving the real issue, add another "problem" trying to workaround it.

About "my proposal": I'm currently working at cleaning up the IT tests, one by one, from unneeded dependencies, then clean up the dependencies tree of that module itself, removing the "install" goal from mvn invoker, and ensure (relying on the standard maven approach - i.e. dependency declaration) that this module is built/executed after all the needed ones (see here) As you may imagine, this would require much more time.

elguardian commented 3 months ago

@gitgabrio

  1. your first consideration is wrong. The local repo will use whatever is in the m2 without getting anything from any remote repo. (that is how you avoid any thing additional to be downlaoded)
  2. it is required in order to override the local repo.
gitgabrio commented 3 months ago

@elguardian

  1. if The local repo will use whatever is in the m2 without getting anything from any remote repo then I fail to understand the need of it
  2. it is required in order to override the local repo sorry, but does not answer my point

Anyway, to cut a long story short, I created another PR that rewrite the module itself, to remove the issue at root:

https://github.com/apache/incubator-kie-kogito-runtimes/pull/3604