You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/05/18 14:46:01 UTC
[GitHub] [camel-k] orpiske opened a new pull request #2311: Check if the environment can produce structured Maven logs before parsing them
orpiske opened a new pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#discussion_r634991871
##########
File path: pkg/util/maven/maven.go
##########
@@ -178,17 +178,24 @@ func Run(ctx Context) error {
Log.Debug("About to start parsing the Maven output")
+ structuredLogs := HasStructuredLog()
+
for scanner.Scan() {
Review comment:
Would trying parsing the first line and falling back to unstructured be a solution? It seems it'd be a more robust heuristic than relying on `MAVEN_OPTS` and `logback.configurationFile` that does not guarantee structured logging is actually enabled.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske commented on pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske commented on pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#issuecomment-843235572
Would you mind a review on this one @squakez ?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske commented on a change in pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske commented on a change in pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#discussion_r634998907
##########
File path: pkg/util/maven/maven.go
##########
@@ -178,17 +178,24 @@ func Run(ctx Context) error {
Log.Debug("About to start parsing the Maven output")
+ structuredLogs := HasStructuredLog()
+
for scanner.Scan() {
Review comment:
Thanks @astefanutti ! I think you are right.
One thing that I did not consider - even in the original implementation is: what if we have an output to the maven log that outputs non-structured bits? In this case, it should go to the normal logger instead of the normalization part.
And, in fact, there are a few scenarios where this could happen (ie.: for example, if the JDK prints a warning, a misbehaved maven plugin bypass the Maven logger, etc).
Let me re-adjust the implementation based on your feedback before we merge this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske edited a comment on pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske edited a comment on pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#issuecomment-843235572
Please, would you mind a review on this one @squakez ?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske commented on a change in pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske commented on a change in pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#discussion_r634998907
##########
File path: pkg/util/maven/maven.go
##########
@@ -178,17 +178,24 @@ func Run(ctx Context) error {
Log.Debug("About to start parsing the Maven output")
+ structuredLogs := HasStructuredLog()
+
for scanner.Scan() {
Review comment:
Thanks @astefanutti ! I think you are right.
One thing that I did not consider - even in the original implementation - is: what if we have an output to the maven log that outputs non-structured bits? In this case, it should go to the normal logger instead of the normalization part.
And, in fact, there are a few scenarios where this could happen (ie.: for example, if the JDK prints a warning, a misbehaved maven plugin bypass the Maven logger, etc).
Let me re-adjust the implementation based on your feedback before we merge this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#discussion_r634991871
##########
File path: pkg/util/maven/maven.go
##########
@@ -178,17 +178,24 @@ func Run(ctx Context) error {
Log.Debug("About to start parsing the Maven output")
+ structuredLogs := HasStructuredLog()
+
for scanner.Scan() {
Review comment:
Would trying parsing the first line and falling back to unstructured be a solution? It seems it'd be a more robust heuristic than relying on `MAVEN_OPTS` and `logback.configurationFile`, that do not guarantee structured logging is actually enabled.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske merged pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske merged pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] orpiske commented on a change in pull request #2311: Check if the environment can produce structured Maven logs before parsing them
Posted by GitBox <gi...@apache.org>.
orpiske commented on a change in pull request #2311:
URL: https://github.com/apache/camel-k/pull/2311#discussion_r634998907
##########
File path: pkg/util/maven/maven.go
##########
@@ -178,17 +178,24 @@ func Run(ctx Context) error {
Log.Debug("About to start parsing the Maven output")
+ structuredLogs := HasStructuredLog()
+
for scanner.Scan() {
Review comment:
Thanks @astefanutti ! I think you are right.
One thing that I did not consider - even in the original implementation - is: what if we have an output to the maven log that outputs non-structured bits? In this case, it should go to the normal logger instead of the normalization part.
And, in fact, there are a few scenarios where this could happen (i.e.: for example, if the JDK prints a warning, a misbehaved maven plugin bypass the Maven logger, etc).
Let me re-adjust the implementation based on your feedback before we merge this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org