dsimansk / camel-undertow

Camel Undertow component
2 stars 0 forks source link

Consider contributing to Apache Camel #1

Open davsclaus opened 9 years ago

davsclaus commented 9 years ago

Hi

How is it going? I wonder if you want to contribute your work to ASF?

We have a ticket about a camel-undertow component https://issues.apache.org/jira/browse/CAMEL-6577

And your work would be a good starting point to get this done?

Are you aware of any work still to be done in this component? And what else you may see as being needed to be done?

dsimansk commented 9 years ago

Hi, of course I was considering contribution to upstream Camel. There was 2 ongoing issue on Undertow[1] and XNIO[2] side to provide both as OSGi bundles. I haven't been following progress on them very closely lately, but they are opened for a year or so. Maybe a gentle push is needed to get them closed. :)

Except that a bit of polishing would be needed to fulfil the contribution requirements of Camel, afaik.

[1] https://issues.jboss.org/browse/UNDERTOW-238 [2] https://issues.jboss.org/browse/XNIO-195

davsclaus commented 9 years ago

Oh its okay for a Camel component not to support OSGi. There is a few that does that.

davsclaus commented 9 years ago

I see this more valuable as a microcontainer way where you bootup Camel with undertow as the http server - not as much for usage in osgi/karaf.

dsimansk commented 9 years ago

Hi, I've prepared initial code for PR. May I proceed to get some feedback on it? https://github.com/dsimansk/camel/tree/undertow

davsclaus commented 9 years ago

Looks great. A few comments

dsimansk commented 9 years ago

PR submitted https://github.com/apache/camel/pull/562

davsclaus commented 9 years ago

Thanks a lot I have merged the code to master branch. I did a few polish and added some TODO what to do still.

The unit tests should really use dynamic port number, we have some port finder code in camel-test that we use in the other http components such as camel-jetty9, camel-http4, camel-ahc or whatnot.

For the async routing engine I may take a look as its a bit complicated for new users. But of course you are welcome to take a stab at that too. You can be inspired in the jetty http producer code that does that.

dsimansk commented 9 years ago

Thanks, I'll look into unit tests and other TODOs and try to do my best.

davsclaus commented 9 years ago

I just pushed the async code change to camel master branch.

As the unit test was actually wrong from the start it surfaces the problem. The producer returns its own response, instead of the response from the server.

So there is a bunch of tests that fails now. You are welcome to take a look

I fixed UndertowComponentTest to be correct.

Also you should

  1. set up mock first,
  2. then send the messages,
  3. and then at the end assert the mock.
dsimansk commented 9 years ago

I think, the problem was that ClientRequest didn't have the path property set. I'll go through rest of the unit tests to implement your recommended steps. https://github.com/dsimansk/camel/commit/20c8b79f2bfd27ea92aec3c05c236b872ac83453

I'll keep this issue open for now.