feat: add kwargs support

[RevinderDev]

Hi,

Goal of this PR was to add support for kwargs arguments to processing function, eg.:

obj.process.activate(my_argument=1)

Doesn't look like the change breaks anything on my end. Let me know if there is something to be improved / missing.

[emil-balashov]

Hi @KasprzykM, sorry for the late reply. Even though celery allows providing some parameters (which could be serialized) into the tasks without any technical problems, it’s been decided to restrict this functionality. Simply saying, it’s easier not to build the validation and error handling around the types which could be serialized or not, as it might lead to some inconvenience, for example, a dict or model instance can be serialized, however, a Django HttpRequest instance cannot be. Therefore, if it’s required to pass any parameters into the celery task, they must be pre-saved first into a model instance or somehow else. If you feel this restriction is limiting your abilities, it’s suggested to explicitly override django_logic_celery.commands.CeleryCommandMixin.get_task_kwargs and provide parameters into a task.

Please, let me know what you think

[RevinderDev]

Hi @emil-balashov, no worries :)

To be honest I kind of expected **kwargs support to be enabled by default for "primitives" (int, bool, strings.. and so on) which is what prompted me to look deeper into it. I never wanted, nor do I think it's a good idea, to serialize complex objects, which is why in provided example I've only given simple integer. It is kind of on me for not directly stating it in a description though :(.

Explicitly overriding django_logic_celery.commands.CeleryCommandMixin.get_task_kwargs is cumbersome approach - subclassing it would mean that I either would have to monkey patch CeleryCommandMixin or define my own classes for Task classes. Wasn't a fan of either of those approaches, hence why PR was made. Unless there is other approach that I am missing?

Correct me if I am wrong, but in case someone did decide to send complex object, wouldn't he just be met with serialization error? Seems like good enough sanity check.

[emil-balashov]

@KasprzykM

expected **kwargs support to be enabled by default for "primitives" (int, bool, strings.

Fair expectation. However, I believe we either have to validate parameters by type (e.g. int, str, etc) and raise an exception otherwise. Also, we can try to disable pickle for celery tasks under Django-logic-celery, but I'm not sure what's the correct way to do that. If you could improve your pull request, I'd be happy to merge, but for the time being, I believe it solves one problem but raises another one.

[RevinderDev]

I have done some initial draft for parameters validations and raising appropriate exception while they don't meet the standard but the problem that I run into is that raising exception in CeleryCommandMixin causes failure_callbacks methods to never fire, which is a big no bueno. Not sure what to do about it, will need to think more. Feel free to close PR or keep it open for a little while :).

Thanks for responding!