[MBUILDCACHE-46] Add maven.build.cache.remote.enabled parameter #43
Conversation
olamy
changed the title
|
created here https://issues.apache.org/jira/browse/MBUILDCACHE-46 |
|
if remote cache is enabled/disabled via configuration does this correctly override it? |
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java
Outdated
Show resolved
Hide resolved
| return defaultValue; | ||
| } | ||
| } | ||
| return Boolean.parseBoolean(value); |
There was a problem hiding this comment.
I think with system properties this is redundant, no?
There was a problem hiding this comment.
This could be accomplished any number of ways, but I decided to go with the micro-optimized form that does not require a string parse to handle the default case.
|
|
||
| // remote build first | ||
| result = findCachedBuild(mojoExecutions, context); | ||
| if (cacheConfig.isRemoteCacheEnabled()) { |
There was a problem hiding this comment.
Please write integration tests for this flag. In pr #34, I added Wiremock-based integration tests to verify the remote cache behavior. The same approach could be reused here. It will be good to see a test that checks that remote is disabled.
There was a problem hiding this comment.
-
For testing flag and xml setting combinations, a lightweight unit test seems prudent. I went ahead and added a unit test for CacheConfigImpl. I tested the initialization and the remote parameters only, but structured it in a way that it should easily accommodate tests around other configurations. A couple integration tests make sense as well, but I thought i'd wait until PR [MBUILDCACHE-25] Fixes N+1 cache processings for forked executions #34 is merged to build off that approach. Tests are a great way to guide meaningful cleanups and refactors, and I feel I need some more time and context to do this at an integration level.
-
Remote configurations are now preconditioned on any meaningful encompassing configuration behind the CacheConfig interface.
There was a problem hiding this comment.
Thank you for the clarifications, I clicked it!
|
Resolve #312 |
1 similar comment
|
Resolve #312 |
6 participants
Use-case:
You may have cases where you want to setup a remote cache, but for whatever reason developers or some build machines do not have access to the server, or have slow internet connection that makes downloading artifacts not sufficiently advantageous over building and caching locally only.
With this parameter, the remote cache can be disabled by default and selectively enabled using a -Dmaven.build.cache.remote.enabled=true flag.
Note:
I still need to get a ASF JIRA account in order to make an issue for this request. I've joined the maven mailing list, but I'm still unclear who to email to request this. There are some issues I'd like to investigate further and potentially contribute PR for on this project.
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MNG-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MNG-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
[ x] I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.