search

instaclick / PDI-Plugin-Step-AMQP

AMQP Plugin for PDI
MIT License
10 stars 14 forks source link

Bind target type, waiting for data consumer, fix storage declare options #10

Closed dshvedchenko closed 9 years ago

dshvedchenko commented 9 years ago

currently Waiting Consumer limited by number of messages, maybe it is need to add timeout field also

dshvedchenko commented 9 years ago

@FabioBatSilva Hello, is there any chances to review it next week ? have a good weekend

FabioBatSilva commented 9 years ago

@dshvedchenko Sorry i didn't review it yet,

I'll try to check it out during the weekend or early next week.. Have a good weekend you too.

FabioBatSilva commented 9 years ago

I like the UI changes you did, But there are a lot of things do configure now, its getting a little confuse. I was wondering if we should create a new tab for bindings, know do you think ?

FabioBatSilva commented 9 years ago

Also I'm not sure how useful is the "Wait for" option, As we discussed before something similar is possible using a "Wait for" step :

"Declare Queue/Bind Step" ->  "Wait for X Sec Step" -> "Queue Consume Step"

Cal you explain what is your use case for this ?

dshvedchenko commented 9 years ago

I think it is more natural way for consume data from queue, probably costs less for CPU. my use case, is to give more freedom for customers of plugin, to expose more features for RMQ client. Specially marked them with red color to point they require more attention when used

dshvedchenko commented 9 years ago

so, current plans for pushing this changes into stable 2.0

FabioBatSilva commented 9 years ago

@dshvedchenko Cool,

let me know when you are done and I'll review/merge it..

dshvedchenko commented 9 years ago

@FabioBatSilva Please, review latest changes

dshvedchenko commented 9 years ago

i've done with changes. now i`m waiting for comments

FabioBatSilva commented 9 years ago

@dshvedchenko Thanks

dshvedchenko commented 9 years ago

@FabioBatSilva Hello, I've seen one commit ahead pull request. I have one concern.

removing QueueingConsumer.Delivery delivery = null; while (!isStopped() && delivery == null && !isStepTimedoutAsConsumer() ) { delivery = consumer.nextDelivery(10000); } ;

completely changed logic of WatingConsumer. Now (if queue is empty) it will wait for one message . My implementation was intended to limit lifetime of whole step. with current implementation this timeout is for time between two messages. if queue will be empty more than timeout, step finished. I think it worth to be mentioned in Readme

FabioBatSilva commented 9 years ago

Hi @dshvedchenko,

That is actually the expected behaviour of a waiting consumer, But is certainly worth documenting the behaviour in the docs anyways.

dbron0000 commented 9 years ago

Hi All,

Compiled the most recent snapshot on Windows (7 64bit) and Linux (ubuntu) AMD64.

Configured a consumer on windows and bound to a queue using the wait for data option. I'm finding that the first event consumed doesn't make it to the next step (although subsequent events are logged (2nd step is write to log).

Anyone seeing anything similar?

DB

On Wed, Apr 1, 2015 at 6:00 PM, Fabio B. Silva notifications@github.com wrote:

Hi @dshvedchenko https://github.com/dshvedchenko,

That is actually the expected behaviour of a waiting consumer, But is certainly worth documenting the behaviour in the docs anyways.

— Reply to this email directly or view it on GitHub https://github.com/instaclick/PDI-Plugin-Step-AMQP/pull/10#issuecomment-88645426 .

dshvedchenko commented 9 years ago

Hi @dbron0000

I will check today, will reply with info in 2 -4 hours

dshvedchenko commented 9 years ago

@dbron0000 checked twice variant:

  1. publish to queue
  2. publish to exchange bound to queue

every time message proceed to second step immediately without loosing.

Please, attach gist with console output of run in rowlevel logging mode

dbron0000 commented 9 years ago

Will do.

Retested with Linux client and not seeing the same behavior. May just be a Windows issue on that workstation.

On Tue, Apr 7, 2015 at 11:24 AM, Denis Shvedchenko <notifications@github.com

wrote:

@dbron0000 https://github.com/dbron0000 checked twice variant:

  1. publish to queue
  2. publish to exchange bound to queue

every time message proceed to second step immediately without loosing.

Please, attach gist with console output of run in rowlevel logging mode

— Reply to this email directly or view it on GitHub https://github.com/instaclick/PDI-Plugin-Step-AMQP/pull/10#issuecomment-90608626 .