search

instaclick / PDI-Plugin-Step-AMQP

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

2.2.1, implement issue 16 #17

Closed dshvedchenko closed 9 years ago

dshvedchenko commented 9 years ago

Please, review proposed changes. uses cases covered

  1. confirmed messages ACK and REJECTED immidiatley if no transaction enabled
  2. if transaction enabled ACK and REJECTED messages form two list of tags. tags ack and Nack at the end
  3. if consumer finished before confirming steps received all data, it will wait for all of them before finish its' work
dshvedchenko commented 9 years ago

@FabioBatSilva ok, got your concern, moved wait to flush() body

dshvedchenko commented 9 years ago

have to reverse , as when ProcessRow signals that step finished, it disconnects its' eventListeners , and it stuck on waiting for steps finished

dshvedchenko commented 9 years ago

remove wait at all

dshvedchenko commented 9 years ago

still have trouble when moving wait from processRow

dshvedchenko commented 9 years ago

i prefer not to move it from ProcessRow, . when I moved kettle log says consumer step finished, but it still waited for ack/nack - and this situation will lead to incorrect understaning flow

dshvedchenko commented 9 years ago

found that kettle writes Finished event to log only after dispose() run, so I blocked dispose with this wait. tested with test-cases:

  1. transactional + no wait for data
  2. transactional + wait for data
  3. transaction + no wait + ack + no reject
  4. transaction + no wait + ack + reject
  5. no wait + ack + reject
  6. wait + ack + reject
FabioBatSilva commented 9 years ago

Cool !!

Now we can get rid of waitForConfirtmationStepsFinished, watchedConfirmStep and confirmStepListener

If you are done, after the clean up I'll do another round of review and we can merge it !!

dshvedchenko commented 9 years ago

I will miss them, please reconsider adding them, as with their presence we provide honest information

FabioBatSilva commented 9 years ago

Can you please configure your editor/ide to use 4 spaces per indentation level and fix the places you are using 2 spaces or tabs ?

dshvedchenko commented 9 years ago

already, did , but in some places edited manualy

dshvedchenko commented 9 years ago

Next steps I propose to split plugin into two steps consumer and publisher and also move to pdi-plugin template structure - like in https://github.com/pentaho/pdi-sdk-plugins.git. I've done AWS SQS like that - and it is much better to build, debug and create in eclipse :)

FabioBatSilva commented 9 years ago

Thanks @dshvedchenko, Nice feature !!

FabioBatSilva commented 9 years ago

Would be nice to split it, i've been planning to do this a while..

Does the template also works with maven ? I know that is standard for pentaho but i hate ant/ivy

dshvedchenko commented 9 years ago

i do not know, i build and debug that plugin with eclipse

FabioBatSilva commented 9 years ago

@dshvedchenko did a whole bunch of refactoring I might want to recompile and see if i didn't break anything..