You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2024/03/22 09:47:44 UTC
[PR] feat(traits): logging refactoring [camel-k]
squakez opened a new pull request, #5270:
URL: https://github.com/apache/camel-k/pull/5270
<!-- Description -->
NOTE: WIP commit to be removed after 3.8.1 is released and before merging this.
Leverage changes introduced in https://github.com/apache/camel-k-runtime/pull/1192
<!--
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
feat(traits): logging refactoring
```
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#discussion_r1535376558
##########
pkg/trait/logging.go:
##########
@@ -80,6 +91,30 @@ func (l loggingTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarQuarkusConsoleColor, True)
}
}
+}
- return nil
+func (l loggingTrait) setCatalogConfiguration(e *Environment) {
+ if e.ApplicationProperties == nil {
+ e.ApplicationProperties = make(map[string]string)
+ }
+ e.ApplicationProperties["camel.k.logging.level"] = l.Level
Review Comment:
I don't think I follow, what I mean is that instead of doing this:
```go
e.ApplicationProperties["camel.k.logging.level"] = l.Level
```
We can leverage the tags defined on the go struct:
```go
e.ApplicationProperties[GetProperty(l.Level)] = l.Level
```
Which can be further improved with reflection.
It should only impact the go code and eventually the documentation but not the CRDs.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#discussion_r1535337743
##########
pkg/trait/logging.go:
##########
@@ -80,6 +91,30 @@ func (l loggingTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarQuarkusConsoleColor, True)
}
}
+}
- return nil
+func (l loggingTrait) setCatalogConfiguration(e *Environment) {
+ if e.ApplicationProperties == nil {
+ e.ApplicationProperties = make(map[string]string)
+ }
+ e.ApplicationProperties["camel.k.logging.level"] = l.Level
Review Comment:
not for this first iteration but we could leverage tags, i.e:
```go
type Logging struct {
Level string `property:"camel.k.logging.level"`
}
```
So the mapping trait -> properties could be made generic and most of the time, not hand crafted.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#discussion_r1535337743
##########
pkg/trait/logging.go:
##########
@@ -80,6 +91,30 @@ func (l loggingTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarQuarkusConsoleColor, True)
}
}
+}
- return nil
+func (l loggingTrait) setCatalogConfiguration(e *Environment) {
+ if e.ApplicationProperties == nil {
+ e.ApplicationProperties = make(map[string]string)
+ }
+ e.ApplicationProperties["camel.k.logging.level"] = l.Level
Review Comment:
not for this first iteration but we could leverage tags, i.e:
```go
type Logging struct {
Level string `property:"camel.k.logging.level"`
}
```
So the mapping trait -> properties could be made generic and most of the time, not manually hadn crafted.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#discussion_r1535575104
##########
pkg/trait/logging.go:
##########
@@ -80,6 +91,30 @@ func (l loggingTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarQuarkusConsoleColor, True)
}
}
+}
- return nil
+func (l loggingTrait) setCatalogConfiguration(e *Environment) {
+ if e.ApplicationProperties == nil {
+ e.ApplicationProperties = make(map[string]string)
+ }
+ e.ApplicationProperties["camel.k.logging.level"] = l.Level
Review Comment:
but this can be deferred to a later stage
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#issuecomment-2031422143
:heavy_check_mark: Unit test coverage report - coverage increased from 37.7% to 37.8% (**+0.1%**)
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5270:
URL: https://github.com/apache/camel-k/pull/5270#discussion_r1535370233
##########
pkg/trait/logging.go:
##########
@@ -80,6 +91,30 @@ func (l loggingTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarQuarkusConsoleColor, True)
}
}
+}
- return nil
+func (l loggingTrait) setCatalogConfiguration(e *Environment) {
+ if e.ApplicationProperties == nil {
+ e.ApplicationProperties = make(map[string]string)
+ }
+ e.ApplicationProperties["camel.k.logging.level"] = l.Level
Review Comment:
Yeah. However the problem there could be how it affects the API. IIUC the conversion would happen when translating the CamelCatalog into a CR. We had included the trait configuration some time ago, making the CRD longer than probably should be. I wonder if adding more stuff may reach some limit that can put us in troubles. In any case, some future challenge we will tackle.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(traits): logging refactoring [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #5270:
URL: https://github.com/apache/camel-k/pull/5270
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org