[JENKINS-60581] Disable download logs in all Maven invocations by default #216
Conversation
| @@ -24,6 +24,8 @@ | |||
| */ | |||
| public class ExternalMavenRunner implements MavenRunner { | |||
|
|
|||
| public static final String DISABLE_DOWNLOAD_LOGS = "-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"; | |||
There was a problem hiding this comment.
isn't --no-transfer-progress / -ntp the preferred option for this now? not sure if you can pass an option through here though
There was a problem hiding this comment.
We do not set high Maven requirements in the tool so far. -ntp option would fail in the current Dockerfile, for example: https://github.com/jenkinsci/plugin-compat-tester/blob/master/Dockerfile
There was a problem hiding this comment.
probably no need for this to be public? even having it as a constant is debate-able?
| public static final String DISABLE_DOWNLOAD_LOGS = "-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"; | |
| private static final String DISABLE_DOWNLOAD_LOGS = "-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"; |
There was a problem hiding this comment.
I agree, i'll change it to private.
There was a problem hiding this comment.
LGTM. I suggest to do Maven requirement bump in a separate pull request.
@stellargo It may make sense to pass this property to all calls tho
| @@ -24,6 +24,8 @@ | |||
| */ | |||
| public class ExternalMavenRunner implements MavenRunner { | |||
|
|
|||
| public static final String DISABLE_DOWNLOAD_LOGS = "-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"; | |||
There was a problem hiding this comment.
We do not set high Maven requirements in the tool so far. -ntp option would fail in the current Dockerfile, for example: https://github.com/jenkinsci/plugin-compat-tester/blob/master/Dockerfile
| @@ -49,6 +51,12 @@ public void run(Config config, File baseDirectory, File buildLogFile, String... | |||
| cmd.add("--define=" + entry); | |||
| } | |||
| cmd.addAll(Arrays.asList(goals)); | |||
| for (String goal: goals){ | |||
There was a problem hiding this comment.
Can be just hardcoded above for all invocations. E.g. like cmd.add("--batch-mode"); above
stellargo
force-pushed
the
disable-dwnld-logs
branch
from
16afacf
to
c50be72
Compare
oleg-nenashev
changed the title
stellargo
force-pushed
the
disable-dwnld-logs
branch
from
c50be72
to
b08652e
Compare
There was a problem hiding this comment.
Thanks a lot @stellargo ! I will go ahead and merge it. It should be released in the new version, maybe in few weeks. Docker images should be delivered by a CD flow within few hours for jenkins/pct:latest
|
Thanks @oleg-nenashev and Tim, couldn't have done it without your help. Looking forward to contribute more :) |
oleg-nenashev
changed the title
| @@ -24,6 +24,8 @@ | |||
| */ | |||
| public class ExternalMavenRunner implements MavenRunner { | |||
|
|
|||
| private static final String DISABLE_DOWNLOAD_LOGS = "-Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"; | |||
There was a problem hiding this comment.
Or -ntp in recent versions of Maven.
…ansferListener=warn is set by jenkinsci/plugin-compat-tester#216
Disable download logs in process-test-classes steps