Add missing parameters to Pipe `log` and `sink` errorHandler

tplevko commented 9 months ago

Describe the Bug

This is more a question regarding the Maximum redeliveries and Redelivery delay - shouldn't these fields be marked as mandatory in the ErrorHandler?

Steps to Reproduce the Bug or Issue

see the log errorHandler configuration

Screenshots or Videos

Screenshot from 2023-11-16 16-23-44

Platform

OS: [e.g. macOS, Windows, Linux]
Browser: [e.g. Chrome, Safari, Firefox]
Version: [latest]

Update

https://github.com/KaotoIO/kaoto-next/issues/356#issuecomment-1910901692 - although these parameters are optional, log and sink Pipe ErrorHandlers are missing more parameters. We should add those.

lordrip commented 9 months ago

sink error handler

tplevko commented 7 months ago

I conducted a research and actually haven't found any information in camel documentation regarding the Maximum redeliveries and Redelivery delay being mandatory. I think, this issue can be closed with wontfix

igarashitm commented 7 months ago

@tplevko can we keep this open? Pipe ErrorHandler schema is tricky that it eventually translates to Camel upstream ErrorHandler

tplevko commented 7 months ago

Sure, no problem, was just sharing my findings, but if there is a reason to have this in place, we can of course leave this open.

igarashitm commented 7 months ago

tl;dr

These parameters are optional. BUT log errorHandler and sink errorHandler have more parameters available.

Details

The Pipe log ErrorHandler is eventually built as a DefaultErrorHandler. When I deploy following with kubectl -f a.pipe.yaml

apiVersion: camel.apache.org/v1
kind: Pipe
metadata:
  name: pipe-2327
spec:
  source:
    properties:
      message: hello
    ref:
      apiVersion: camel.apache.org/v1
      kind: Kamelet
      name: timer-source
  sink:
    ref:
      apiVersion: camel.apache.org/v1
      kind: Kamelet
      name: log-sink
  errorHandler:
    log:
      parameters: {}

Then I can see it's assigned to DefaultErrorHandlerBuilder by kamel describe integration pipe-2327

...
Configuration:
  Type: property
  Value:    camel.beans.defaultErrorHandler = #class:org.apache.camel.builder.DefaultErrorHandlerBuilder
  Type: property
  Value:    camel.k.errorHandler.ref = defaultErrorHandler
...

So redeliveryDelay and maximumRedeliveries are assigned through

On the other hand, since DefaultErrorHandler itself and redelivery policy don't have required field, in theory all the options are optional.

BUT,

More parameters exist on log and sink errorHandler

Although at first I added only these 2 parameters into the schema by looking at this example https://camel.apache.org/camel-k/next/kamelets/kameletbindings-error-handler.html

In fact the log errorHandler is built with DefaultErrorHandlerBuilder, and also the sink errorHandler is built with DeadLetterChannelBuilder , therefore all the parameters these builder support could be specified. We need to update the schema and add those parameters. Note that there's no official schema for Pipe ErrorHandler. Because of that we're using a custom made one in Kaoto. Unfortunately it's even different from DefaultErrorHandlerDefinition in camelYamlDsl, for example redeliveryDelay is not a direct parameter but is specified through redelivery policy in camelYamlDsl.

Said custom made schema file is here - https://github.com/KaotoIO/kaoto-next/blob/main/packages/camel-catalog/assembly/src/main/resources/schemas/PipeErrorHandler.json

KaotoIO / kaoto