camunda-community-hub / camunda-platform-7-camel

Community Extension to add Apache Camel support for Camunda Platform 7
Apache License 2.0
82 stars 57 forks source link

External tasks consumer #24

Closed RasPelikan closed 7 years ago

RasPelikan commented 8 years ago

Hi Bernd,

In the recent past I used Camel integration to talk to a we service out of a service task. In this situation I also correlated subsequent incoming webservice calls by using the Camel integration. Everything was fine. Then I did load tests and suddenly correlation did not work reliable. The reason was Camunda's worker thread pool: the threads were occupied by web service communication and were not able to serve processing workflows. So the process was not in a receive-task but it should be. I increased the thread pool according the load and it worked again. I thought to myself: this would be a perfect situation for external tasks! So I decided to add external task polling to Camunda's Camel integration since Camel brings a lot of functionality out of the box.

I hope you like my extension and maybe it is a good thing to mention in your upcoming Webinar about external tasks ;-)

Stephan

RasPelikan commented 7 years ago

Hi Bernd,

I did the refactoring. Additionally I added support for 7.6.0 by introducing a new parameter "deserializeVariables" (see last commits). From my point of view everything is ready for a new release :-)

Stephan

RasPelikan commented 7 years ago

Hi Bernd,

I added support for async processors. The intention was to process external tasks in parallel. But even for async processors Camel's ScheduledBatchPollingConsumer waits until they are done. So after some investigation I could figure out how to process tasks in parallel and put this information into the README.md.

I hope you will soon merge the pull request ;-)

Stephan

berndruecker commented 7 years ago

Hi Stephan.

Sorry for the long delay! I had a quick look and decided to merge it and afterwards do a couple of own tests and validations. Afterwards I can release a version 0.5 - or is there anything missing from your perspective?

Cheers Bernd

berndruecker commented 7 years ago

Please do not change formatting of files next time! And quick question: Why did you commented the genration of sources in Maven?

berndruecker commented 7 years ago

You introduced a hard requirement for Java 8 - it does not compile with Java 7 anymore:

14:29:36 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project camunda-bpm-camel-common: Compilation failure 14:29:36 [ERROR] /var/lib/jenkins/jobs/community-extension-camel-DISTRO-RELEASE/workspace/camunda-bpm-camel-common/src/main/java/org/camunda/bpm/camel/component/externaltasks/BatchConsumer.java:[204,45] no suitable constructor found for PriorityQueue(<anonymous java.util.Comparator>)

I don't think we should require Java 8 without very good reasons. Can you please adjust this, so the component can also be build with Java 7?

Thanks. Please open an own PullRequest for it.

RasPelikan commented 7 years ago

Why did you commented the genration of sources in Maven?

This does not work in Eclipse. I forgot to de-comment it again. I added an Eclipse lifecycle-mapping into the parent pom. I hope this is OK to you. It is ignored in other environments.

You introduced a hard requirement for Java 8 - it does not compile with Java 7 anymore

This was unintended. I fixed this.

I'll send another pull request.

It will also include another "fix": For async responses it may happen that the external task is not available any more (e.g. the (embedded-sub)process was cancelled due to a interrupting timer, etc.). In this situation a NPE was thrown. I changed this to a NoSuchExternalTaskException (derived from ).

berndruecker commented 7 years ago

What does this lifecycle mapping do - why did you introduce it?

RasPelikan commented 7 years ago

This tells Eclipse on importing this maven project how to handle maven plugins not nativly supported by Eclipse. It is ignored in any other environment. As it does not effect the build I hoped you may accept it since it helps to avoid committing changes (as happend before) made to get it run within Eclipse.

berndruecker commented 7 years ago

I am not a big fan of vhaving eclipse specific things in the pom - as this is "only" your local environment. But I do not mind that much to delay merging because of it. As far as I can see it does not do any harm.