-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[test] add Kamelet errorHandler test #2522
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a very good PR that includes the Kamelet creation support! There are a few changes that would make it more reusable, feel free to follow those suggestions and if you have any further doubt I will be happy to help.
b9a4721
to
2df7788
Compare
@squakez, @astefanutti the CI fails with
the generated KameletBinding is
The most important message is that the test is working on |
Yeah, I bumped into that issue as well while working on something different. I am investigating and will report more details shortly. |
Tracked here #2525 |
Let's wait to merge #2564 and we can rebase this. |
#2564 is merged - can we proceed with the rebase? |
Yes please. |
6b0b2c3
to
25318e7
Compare
@squakez Two CI workflows failed but it seems that it's not my fault. Can you rerun them? |
e2e/support/test_support.go
Outdated
Ref: &to, | ||
Properties: asEndpointProperties(sinkProperties), | ||
}, | ||
ErrorHandler: asErrorHandlerSpec(errorHandler), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one should be optional. If you provide some meaningful configuration, then you can provide the ErrorHandler
parameter, otherwise you must not. The error reported in the CI may be because of this configuration that likely is set to empty.
25318e7
to
581e8bb
Compare
Release Note