introducing KnownResult for WS

[OmarWKH]

Following from #51.

This is one way for the WS backend to avoid AsyncResult but still use Progress.get_info.

Progress.get_info can take either an AsyncResult or a KnownResult. WS code would use a KnownResult.

If this is ok, it should be tested with #51 before merging.

[EJH2]

It seems like you built this off of master, so I will have to re-do all of the proposed changes from #51 to incorporate this, which may complicate merging either.

[OmarWKH]

I merged the PRs.

There's an issue when retrying tasks because Progress.get_info returns an exception instead of a dictionary.

For HTTP it's not relevant to the PR (#53).

For WS, I'm not sure. On master, task_postrun_handler raises TypeError("'StopIteration' object is not a mapping") as expected. I think you were working on improving this behaviour in your branch, right?

On this branch, the post handler does not run. I'm not sure why. Celery logs this:

Could not process signal receiver <function task_postrun_handler at 0x7f809fb745e0>. Retrying in 2.00 seconds...

[EJH2]

There seems to be a previously undiscovered edge-case involving retries right now, as well as ignored tasks. The signal error is most likely because the Retry error that celery throws cannot be serialized by JSON. It seems that AsyncResult does something to bypass the problem, and I'm keen on investigating it. For now, #51 got around this problem by casting everything to string to prevent a serialization error, and in turn killing out the signal. The master branch also got around this because it didn't rely on the signal's data. Unfortunately, if I can't figure out a way to properly handle it in the signal, we may have to dump that part of the changes all together.

[EJH2]

For WS, I'm not sure. On master, task_postrun_handler raises TypeError("'StopIteration' object is not a mapping") as expected. I think you were working on improving this behaviour in your branch, right?

I don't think I intended that to be the error, rather it was a small oversight that the Progress dictionary gets unpacked and re-packed in another dictionary object, versus just serving up the same dictionary. Removing the dictionary unpack would change the error to TypeError: can not serialize 'StopIteration' object, which would probably make more sense than the current error. Part of the reason the original error exists anyway is because of the issue outlined in #54, but hopefully this will serve as a jumping-off point to find a solution to that as well.

EDIT: I've pushed a commit removing the dictionary unpack to be more in line with the HTTP view

[czue]

@EJH2 happy to let you handle review of this one since it is most relevant to your recent work.

[EJH2]

@OmarWKH Once you merge the rest of the changes from #51 into here, I can start testing to make sure this is stable enough to merge before we start tackling #53 and #54.

[OmarWKH]

@EJH2 done

I made Progress.get_info always return serializable data so the behaviour is more predictable even before fixing the retry issue.

[EJH2]

Looks good to me. Only nitpick I have is using double quotes for docstrings vs single quotes. Otherwise, this is good to go for a merge.

[OmarWKH]

Merged because @czue deferred the review to @EJH2.

[czue]

Yep, this is great, thanks for handling it!

Would this be a good time to cut a new release? Or do you think we should wait for #53 or #54 to be addressed first

[OmarWKH]

A new release would be good for WS users. I'm not sure when #53/#54 will be fully addressed.

[czue]

Will do. Want to get #57 in and then will push to pypi (everything else looking good on my side)

[czue]

Just released 0.0.13! https://pypi.org/project/celery-progress/0.0.13/

As an aside... I think maybe I'll start changing the release versions to 0.1, 0.2 moving forwards... maybe we can even get to 1.0! 🙃

[czue]

FYI I somehow botched the packaging on 0.0.13, so released a fixed 0.0.14. 🤦‍♂️