save extended properties only when asked for

[AmitPhulera]

Fixes https://github.com/celery/django-celery-results/issues/315

Currently django-celery-results stores all the info about the task to DB whereas the default behavior as per celery should only be storing some basic properties. Extended properties should be saved only when they are requested for. It can be done by setting extended-result flag in the celery config.

The PR checks for that flag while storing the results and proceeds accordingly.

I would be adding some tests for it once we are okay with the approach.

[AmitPhulera]

@auvipy Thanks for the quick turnaround. Yeah, I am planning to add unit tests for it. Just wanted to be sure that this is the right way to go forward.

[AmitPhulera]

Hey @auvipy, I have added a test to verify the flag works https://github.com/celery/django-celery-results/pull/316/commits/7edbd42250f751b654108fa55e09ba714d4b6efd and also updated previous tests to use the flag 6974a76

[AmitPhulera]

Hey @auvipy, Could you please let me know what would be the next steps in this PR?

[auvipy]

@AllexVeldman can you check this please

[AmitPhulera]

Thanks for your review @AllexVeldman, I have addressed the feedback that you have shared. Let me know if there is anything else that you want me to update.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging bde3eefc74bc6cd91b860bb032184c6b7bb0b247 into e174c9978c66f7d9c753d9332e2ec66c8abe1c1a - view on LGTM.com

new alerts:

• 1 for Signature mismatch in overriding method

[AmitPhulera]

@auvipy Can you please guide me on what should be the correct place to document it? The celery docs already have information about the same. Do you think an entry in changelog would suffice?

[auvipy]

If you can please dig the docs and find out any suitable place to add, otherwise an entry to changelog would be alright. but I would request to check the docs more initially.

[AmitPhulera]

@auvipy I was actually looking into the docs earlier to figure out if this can be communicated in a better way. Still, the only docs that I came across were these which mention the installation(which points to celery docs), API, and changelog.

I think only entry in changelog makes sense here

If you have any other place in mind, I would be happy to make the changes there.

[auvipy]

ok put in change log please

[AmitPhulera]

@auvipy I have added the changes in Changelog. See https://github.com/celery/django-celery-results/pull/316/commits/af09b8784cebb1014dd7a0b924168a84dfab1bf1

I have put it in the minor version bump rather than updating the patch version. Feel free to update as you seem fit.

[AmitPhulera]

Hey @auvipy, Would you be able to provide an estimate on when a release tag would be created for the change?

[auvipy]

this week or week end

[auvipy]

this creates a regression, unless their is a fix, going to revert this

[AmitPhulera]

@auvipy Can you please elaborate the issue?

[auvipy]

new update https://github.com/celery/django-celery-results/issues/326#issuecomment-1177478454

[AmitPhulera]

I guess the change is okay, right?

[auvipy]

seems yeah, need some doc direction most probably

[valberg]

Could we add a FAQ or something to the docs to inform of this? Maybe even copy the "Getting started" from https://docs.celeryq.dev/en/latest/django/first-steps-with-django.html#django-celery-results-using-the-django-orm-cache-as-a-result-backend and include a note in that?

[auvipy]

please go ahead

[valberg]

Here you go :) https://github.com/celery/django-celery-results/pull/328

[G-Rath]

Looks like this resolves #142 aka GHSA-fvx8-v524-8579 🎉

[auvipy]

wow