You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "fil512 (via GitHub)" <gi...@apache.org> on 2023/11/01 17:52:20 UTC
[PR] Converter lookup once again matches subclass converters like in Camel 3 [camel]
fil512 opened a new pull request, #11882:
URL: https://github.com/apache/camel/pull/11882
This is a fix to what looks like an accidental regression in Camel 3 -> Camel 4.
Fix for https://issues.apache.org/jira/browse/CAMEL-19828
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1793377512
> Those tests depended on the original behaviour. I restored that behaviour. Can you run the test suite again?
I triggered a test on my CI and I'll check the results on Monday. I am not sure if that's the right approach, though ... but let's see how it works.
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1790597620
@fil512 as it is, this patch breaks `camel-jackson`. It can be fixed by adding a strict converter on [JacksonTypeConverter](https://github.com/apache/camel/blob/main/components/camel-jackson/src/main/java/org/apache/camel/component/jackson/converter/JacksonTypeConverters.java#L172) that can match a BooleanNode type.
```
@Converter
public Boolean toBoolean(BooleanNode node, Exchange exchange) throws Exception {
return node.asBoolean();
}
```
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1792571881
Something is still off with this one as I see a couple of tests failing:
| Failed Test | Duration | Failure Type |
| --- | --- | --- |
| org.apache.camel.builder.xml.XPathTest.testXPathNotUsingExchangeEvaluate | 0.076s | ERROR |
| org.apache.camel.builder.NotifyBuilderTest.testFromFilterBuilderWhenExchangeDone | 0.135s | FAILURE |
| org.apache.camel.component.jackson.converter.JacksonJSonNodeConverterTest.convertToBoolean | 0.021s | ERROR |
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1798352293
Worked OK on my own CI. Let's get this one in.
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "fil512 (via GitHub)" <gi...@apache.org>.
fil512 commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1790888638
Fixed.
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1789398824
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: Apache Camel Committers, please review the following items:
* First-time contributors **require MANUAL approval** for the GitHub Actions to run
* You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot.
* You can label PRs using `build-all`, `build-dependents`, `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
* Build and test logs are available in the Summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
* :warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "fil512 (via GitHub)" <gi...@apache.org>.
fil512 commented on code in PR #11882:
URL: https://github.com/apache/camel/pull/11882#discussion_r1379188114
##########
core/camel-core/src/test/java/org/apache/camel/impl/converter/TypeResolverHelperTest.java:
##########
@@ -0,0 +1,76 @@
+package org.apache.camel.impl.converter;
Review Comment:
fixed
##########
core/camel-core/src/test/java/org/apache/camel/impl/converter/TypeResolverHelperTest.java:
##########
@@ -0,0 +1,76 @@
+package org.apache.camel.impl.converter;
+
+import org.apache.camel.TypeConverter;
+import org.apache.camel.converter.TypeConvertible;
+import org.apache.camel.support.SimpleTypeConverter;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.*;
Review Comment:
fixed
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "essobedo (via GitHub)" <gi...@apache.org>.
essobedo commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1790563234
Github action `component-test` doesn't work properly in this revision, a rebase on main is required to get the latest changes
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1790551639
/component-test camel-jackson
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #11882:
URL: https://github.com/apache/camel/pull/11882
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on code in PR #11882:
URL: https://github.com/apache/camel/pull/11882#discussion_r1379168714
##########
core/camel-core/src/test/java/org/apache/camel/impl/converter/TypeResolverHelperTest.java:
##########
@@ -0,0 +1,76 @@
+package org.apache.camel.impl.converter;
Review Comment:
Missing license header
##########
core/camel-core/src/test/java/org/apache/camel/impl/converter/TypeResolverHelperTest.java:
##########
@@ -0,0 +1,76 @@
+package org.apache.camel.impl.converter;
+
+import org.apache.camel.TypeConverter;
+import org.apache.camel.converter.TypeConvertible;
+import org.apache.camel.support.SimpleTypeConverter;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.*;
Review Comment:
Don't use * imports
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1794550752
> > Those tests depended on the original behaviour. I restored that behaviour. Can you run the test suite again?
>
> I triggered a test on my CI and I'll check the results on Monday. I am not sure if that's the right approach, though ... but let's see how it works.
There was an outage on the lab, so this job did not finish. I am running it again and will report once I have the results.
--
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] Converter lookup once again matches subclass converters like in Camel 3 [camel]
Posted by "fil512 (via GitHub)" <gi...@apache.org>.
fil512 commented on PR #11882:
URL: https://github.com/apache/camel/pull/11882#issuecomment-1792641278
Those tests depended on the original behaviour. I restored that behaviour. Can you run the test suite again?
--
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