-
Notifications
You must be signed in to change notification settings - Fork 185
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
Camel quarkus netty #353
Camel quarkus netty #353
Conversation
integration-tests/netty/src/main/java/org/apache/camel/quarkus/component/netty/CamelRoute.java
Outdated
Show resolved
Hide resolved
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.
Minor stuff, I guess you'll need to run the build with sourcecheck profile enabled and check for warnings
And by the way you have conflicts need to be fixed. |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
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.
Good work, thanks!
Some comments inline. Please rebase and squash your commits.
extensions/netty/runtime/src/main/resources/META-INF/quarkus-extension.json
Outdated
Show resolved
Hide resolved
integration-tests/netty/src/main/resources/application.properties
Outdated
Show resolved
Hide resolved
integration-tests/netty/src/main/resources/application.properties
Outdated
Show resolved
Hide resolved
...gration-tests/netty/src/test/java/org/apache/camel/quarkus/component/netty/it/NettyTest.java
Outdated
Show resolved
Hide resolved
...gration-tests/netty/src/test/java/org/apache/camel/quarkus/component/netty/it/NettyTest.java
Outdated
Show resolved
Hide resolved
Refer to this link for build results (access rights to CI server needed): |
Thanks for pointing this out. It's not documented. |
Refer to this link for build results (access rights to CI server needed): |
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.
More comments inline.
integration-tests/netty/src/test/java/org/apache/camel/quarkus/component/netty/it/NettyIT.java
Outdated
Show resolved
Hide resolved
True. Let me doc it #354 |
.gitignore
Outdated
@@ -19,7 +19,7 @@ bin/ | |||
*.iws | |||
|
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'm always for removing the gitignore file.
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.
More comments inline.
...gration-tests/netty/src/test/java/org/apache/camel/quarkus/component/netty/it/NettyTest.java
Outdated
Show resolved
Hide resolved
integration-tests/netty/src/main/java/org/apache/camel/quarkus/component/netty/CamelRoute.java
Show resolved
Hide resolved
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
e4a98a6
to
75a28d0
Compare
75a28d0
to
7bb74e5
Compare
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 1.58 MB...] at sun.reflect.GeneratedMethodAccessor237.invoke (Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke (Method.java:498) at io.quarkus.deployment.ExtensionLoader$1.execute (ExtensionLoader.java:941) at io.quarkus.builder.BuildContext.run (BuildContext.java:415) at org.jboss.threads.ContextClassLoaderSavingRunnable.run (ContextClassLoaderSavingRunnable.java:35) at org.jboss.threads.EnhancedQueueExecutor.safeRun (EnhancedQueueExecutor.java:2011) at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask (EnhancedQueueExecutor.java:1535) at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run (EnhancedQueueExecutor.java:1426) at java.lang.Thread.run (Thread.java:748) at org.jboss.threads.JBossThread.run (JBossThread.java:479)[ERROR] [ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :camel-quarkus-integration-test-corechannel stoppedAdding one-line test results to commit status...Setting status of e4a98a6 to FAILURE with url https://builds.apache.org/job/camel-quarkus-pr/369/ and message: 'FAILURE 96 tests run, 2 skipped, 0 failed.'Using context: default |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Adding comment to this ticket (already shared on https://gitter.im/apache/camel-quarkus), had some challenges with the rebase/squash operations and instead needed to recover and re-commit the branch, as such some of the references in the code reviews may unfortunately not work as expected. |
Refer to this link for build results (access rights to CI server needed): |
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.
Thanks for the updates!
Some comments inline.
Please run mvn clean install -DskipTests
from the root dir once again and amend the changes in list-of-camel-quarkus-extensions.adoc
There are some duplications between netty and netty-http components now. In a follow up step, we should make netty-http dependent on netty and remove the dups. We should also document in the netty.adoc
that the native transport is not supported in the native mode. I'll create issues for these.
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.
Thanks. Just two details: (1) remove camel-endpointdsl
from poms/bom/pom.xml
and (2) squash your commits. Otherwise, the PR is good to merge.
ce44366
to
a2bed50
Compare
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.
Great, thanks!
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.
Please remove the gitignore
Sorry just realised it's a gitignore already there. Sorry. Looks good |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
#248 initial support for camel-quarkus-netty