RetailMeNotSandbox / dart

Self-service data workflow management
MIT License
17 stars 12 forks source link

consume_subscription and trigger worker purge hotfix #164

Closed maybeiambatman closed 7 years ago

ophiradi commented 7 years ago

My only pet peeve is that you are sending subscription_element_service instead of subscription_element_service.assign_subscription_elements. That makes unit testing harder. But it is not important.

More specific question, does calling self.subscription_element_service.assign_subscription_elements before actually completing the action makes sense? what happens if the action fails?

maybeiambatman commented 7 years ago

@ophiradi

I sent the subscription_element_service because the assign_subscription_elements method is an instance method. I wouldn't be opposed to refactoring the method such that it's a staticmethod. But that might mean investigating more time into finding other possible references to it.

And yes, it does make sense. If an action fails, it is handled on when checking in an action after it's failed or completed successfully. See the following for reference:

https://github.com/RetailMeNotSandbox/dart/blob/master/src/python/dart/web/api/engine.py#L109 https://github.com/RetailMeNotSandbox/dart/blob/master/src/python/dart/service/workflow.py#L172