search

celery / django-celery-results

Celery result back end with django
Other
696 stars 206 forks source link

fix: allow for custom states in status field #407

Open ericswpark opened 1 year ago

ericswpark commented 1 year ago

From https://github.com/celery/django-celery-results/pull/366#issuecomment-1586237389

Thanks to @cdevacc1 for the idea. I just decided to write up the PR as it only takes a couple of minutes. This should resolve the bug where custom states are not shown at all in the admin panel.

auvipy commented 1 year ago

And tests are already failing you can see that

ericswpark commented 1 year ago

Yup, forgot to create DB migrations after the change. (unless it was failing for a different reason)

ericswpark commented 1 year ago

@auvipy the tests should now pass with the added migration. Can I please request a review? Thanks!

ericswpark commented 5 months ago

@auvipy I'm not sure what breaking changes you're concerned about -- removing the line should allow for custom states while letting those who originally used the TASK_STATE_CHOICES for choices keep their data with no changes necessary. Please let me know if there's anything else you need or any other changes that should be made before the PR is merged or rejected. Thanks.

ericswpark commented 5 months ago

@auvipy may this please be reviewed before it becomes stale? I'd like to avoid having to rebase it over and over and this PR's been sitting in my list since last year. Thanks!

auvipy commented 5 months ago

@auvipy I'm not sure what breaking changes you're concerned about -- removing the line should allow for custom states while letting those who originally used the TASK_STATE_CHOICES for choices keep their data with no changes necessary. Please let me know if there's anything else you need or any other changes that should be made before the PR is merged or rejected. Thanks.

my only concern is if existing users are going to miss anything by the removal of the TASK_STATE_CHOICES

ericswpark commented 5 months ago

my only concern is if existing users are going to miss anything by the removal of the TASK_STATE_CHOICES

Please see previous comments. It should not affect existing users as it only limits what choices can go inside the field. Removing it expands it to allow for other values, not just the ones in TASK_STATE_CHOICES.