squakez commented on code in PR #5245:
URL: https://github.com/apache/camel-k/pull/5245#discussion_r1522827198
##########
pkg/apis/camel/v1/integration_types_support.go:
##########
@@ -116,6 +116,11 @@ func (in *IntegrationSpec) AddDependency(dependency
string) {
in.Dependencies = append(in.Dependencies, dependency)
}
+// AddConfigurationProperty adds a new configuration property.
+func (in *IntegrationSpec) AddConfigurationProperty(confValue string) {
Review Comment:
`.integration.spec.configuration` is deprecated. We should not use it any
longer.
##########
pkg/trait/error_handler_test.go:
##########
@@ -85,9 +85,9 @@ func TestErrorHandlerApplyDependency(t *testing.T) {
CamelCatalog: c,
Integration: &v1.Integration{},
}
- e.Integration.Spec.AddConfiguration("property",
"camel.beans.defaultErrorHandler =
#class:org.apache.camel.builder.DeadLetterChannelBuilder")
- e.Integration.Spec.AddConfiguration("property",
"camel.beans.defaultErrorHandler.deadLetterUri = log:info")
- e.Integration.Spec.AddConfiguration("property", fmt.Sprintf("%v = %s",
v1.ErrorHandlerRefName, "defaultErrorHandler"))
+
e.Integration.Spec.AddConfigurationProperty("camel.beans.defaultErrorHandler =
#class:org.apache.camel.builder.DeadLetterChannelBuilder")
Review Comment:
Maybe we can take the opportunity and move into the non deprecated logic
also these conf.
##########
pkg/trait/error_handler.go:
##########
@@ -67,6 +69,17 @@ func (t *errorHandlerTrait) Apply(e *Environment) error {
t.addErrorHandlerDependencies(e, defaultErrorHandlerURI)
}
+ if shouldHandleNoErrorHandler(e.Integration) {
+ // noErrorHandler is enabled by default on Kamelets
since Camel 4.4.0 (runtimeVersion 3.8.0)
+ // need to disable this setting so that pipe error
handler works
+ confValue, err :=
property.EncodePropertyFileEntry("camel.component.kamelet.noErrorHandler",
"false")
+ if err != nil {
+ return err
+ }
+
+ e.Integration.Spec.AddConfigurationProperty(confValue)
Review Comment:
You need to add the property into the Environment and make sure this is
called before the configmap holding `application.properties` is generated.
##########
pkg/trait/error_handler.go:
##########
@@ -67,6 +69,17 @@ func (t *errorHandlerTrait) Apply(e *Environment) error {
t.addErrorHandlerDependencies(e, defaultErrorHandlerURI)
}
+ if shouldHandleNoErrorHandler(e.Integration) {
+ // noErrorHandler is enabled by default on Kamelets
since Camel 4.4.0 (runtimeVersion 3.8.0)
+ // need to disable this setting so that pipe error
handler works
+ confValue, err :=
property.EncodePropertyFileEntry("camel.component.kamelet.noErrorHandler",
"false")
Review Comment:
I think the main problem with this is that we are adding a patch, instead of
handling the default value as the new Camel is meant to be. For this reason,
IMO, we should manage this at runtime level. In other rejected PRs we have
already mentioned these aspect should not concern the operator and should be
managed by the runtime. Let's try to be consistent then.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
