Skip to content
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

Consider contributing to Apache Camel #1

Open
davsclaus opened this issue Jul 11, 2015 · 10 comments
Open

Consider contributing to Apache Camel #1

davsclaus opened this issue Jul 11, 2015 · 10 comments

Comments

@davsclaus
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@davsclaus
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

Looks great. A few comments

  • we do not use @author tags in javadoc at ASF
  • the undertow producer may benefit from using the camel async routing engine - I can help with that after the initial donation - but you use the boolean process(exchange, callback) method and then invoke the done method on the callback when you have the response ready. Though there is a little bit more to it than that.

@dsimansk
Copy link
Owner

PR submitted apache/camel#562

@davsclaus
Copy link
Author

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
Copy link
Owner

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

@davsclaus
Copy link
Author

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
Copy link
Owner

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.
dsimansk/camel@20c8b79

I'll keep this issue open for now.

@dsimansk dsimansk reopened this Jul 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants