search

reverbdotcom / reverb-magento

Magento 1.x plugin for syncing with Reverb
Other
7 stars 10 forks source link

When saving magento product, reverb sync should be backgrounded #207

Closed skwp closed 8 years ago

skwp commented 8 years ago

This seems strange - if reverb is slow/down it will affect the magento save of the product in the catalog. Is this how other plugins are done? I think we should be backgrounding this

cc @zztimur @dunagan5887 @StevenWolfe

dunagan5887 commented 8 years ago

This was done this way partially due to the legacy code that we were working with/around. Currently the Bulk Product sync button queues up background tasks to sync listings. We could have that same functionality queue up a single product's listing in a background task. This would be a 1-2 hour max

zztimur commented 8 years ago

@dunagan5887 for some reason bulk sync doesn't work for me. I haven't checked why yet

skwp commented 8 years ago

Yes I think this should be done thanks

Sent from my iPhone

On Feb 18, 2016, at 6:28 PM, Sean Dunagan notifications@github.com wrote:

This was done this way partially due to the legacy code that we were working with/around. Currently the Bulk Product sync button queues up background tasks to sync listings. We could have that same functionality queue up a single product's listing in a background task. This would be a 1-2 hour max

— Reply to this email directly or view it on GitHub.

skwp commented 8 years ago

@zztimur is your cron ok?

skwp commented 8 years ago

@dunagan5887 this is at the top of my list, the only one I'm concerned about right now

dunagan5887 commented 8 years ago

@skwp Should I also background the product listing sync during the Order Save?

skwp commented 8 years ago

yes we should not block magento in general from normal path operations

dunagan5887 commented 8 years ago

There was the following comment in the code which referenced exceptions thrown while executing the product listing sync during product save

// Any other Exception is understood to prevent product save

Do we want the failure to queue a listing sync to prevent product save? (There's no normal reason this would happen, it would signify a serious issue with the module or Magento instance)

Given this update, an exception during background processing of the listing sync will no longer prevent product save

skwp commented 8 years ago

This is correct, our plugin should never interfere with a customer's critical path to saving a product for their own purposes. The background task can be retried at a later time

skwp commented 8 years ago

i.e. whatever code is there should be considered legacy; we should not prevent a product save by any means

dunagan5887 commented 8 years ago

@skwp There was logic in place which prevented listing creations from being executed during the order placement event observer (listings would only be updated). Should this logic remain?

dunagan5887 commented 8 years ago

@zztimur The Bulk Sync is working on my local, it's queue-ing up image listing syncs as well. Let me know what you find if you look into it

zztimur commented 8 years ago

Images don't seem to sync. figuring it out. from my logs:

2016-02-25T01:21:14+00:00 ERR (3): exception 'Exception' with message 'Error executing task for queue task object with id 535: The API call returned an HTTP status that was not 200: 502. URL: https://sandbox.reverb.com/api/my/listings?state=all&sku=ODS-060 Content: Curl Error Message: ' in /chroot/home/xxx/xxxx/html/.modman/reverb-magento/app/code/community/Reverb/ProcessQueue/Helper/Task/Processor.php:230 Stack trace:

0 /chroot/home/xxx/xxxxx/html/.modman/reverb-magento/app/code/community/Reverb/ProcessQueue/Helper/Task/Processor.php(102): Reverb_ProcessQueue_Helper_Task_Processor->_logError('Error executing...')

1 /chroot/home/xxxx/xxx/html/.modman/reverb-magento/app/code/community/Reverb/ProcessQueue/Helper/Task/Processor.php(36): Reverb_ProcessQueue_Helper_Task_Processor->processQueueTask(Object(Reverb_ProcessQueue_Model_Task_Unique))

2 /chroot/home/xxx/xxxx/html/.modman/reverb-magento/app/code/community/Reverb/ReverbSync/Model/Cron/Listings/Images/Sync.php(19): Reverb_ProcessQueue_Helper_Task_Processor->processQueueTasks('listing_image_s...')

skwp commented 8 years ago

@zztimur that looks like a temporary sandbox issue. are you getting that repeatedly? can I see the reverb curl log?

@dunagan5887 i'm not sure what you mean by listing creations being blocked during order placement. what is the impact of that? what do orders have to do with listings?

dunagan5887 commented 8 years ago

When an order is placed, there was an event observer which executed a listing sync with reverb (presumably to update the inventory count on Reverb). If the listing had already been created on Reverb it would be updated. However if the sku had not been listed on Reverb, the listing would not be created as a result of this order placement observer

skwp commented 8 years ago

Yes that seems like a bug. If listing creation is enabled in settings then any update to reverb should first check if the listing exists and if not, create it. All updates regardless of whether they are order syncs or anything else should go through this flow.

zztimur commented 8 years ago

I'll send you errors (if any) once we test it again.

On Thu, Feb 25, 2016 at 10:04 AM, Yan Pritzker notifications@github.com wrote:

Yes that seems like a bug. If listing creation is enabled in settings then any update to reverb should first check if the listing exists and if not, create it. All updates regardless of whether they are order syncs or anything else should go through this flow.

— Reply to this email directly or view it on GitHub https://github.com/reverbdotcom/reverb-magento/issues/207#issuecomment-188854274 .

-- Sent from my iPhone