Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid crash when can not get human readable description #648

Merged
merged 1 commit into from Apr 8, 2023
Merged

Avoid crash when can not get human readable description #648

merged 1 commit into from Apr 8, 2023

Conversation

nijel
Copy link
Contributor

@nijel nijel commented Mar 27, 2023

Catch errors from cron-descriptor and display crontab as it is. Apparently there are crontabs which work in Celery, but cron-descriptor fails to format them.

Issue #647

Catch errors from cron-descriptor and display crontab as it is.
Apparently there are crontabs which work in Celery, but cron-descriptor
fails to format them.

Issue #647
@auvipy auvipy self-requested a review March 29, 2023 17:27
@auvipy
Copy link
Member

auvipy commented Mar 29, 2023

@Salamek can you have a look please?

@Salamek
Copy link

Salamek commented Mar 29, 2023

@auvipy It is ok as a hotfix (to prevent a crash)... but i think you should create a valid CRON expression from crontab object and pass it to cron_descriptor to get correctly displayed...

@auvipy
Copy link
Member

auvipy commented Mar 30, 2023

but i think you should create a valid CRON expression from crontab object and pass it to cron_descriptor to get correctly displayed...

is celery doing this ? or this package need update as well?

@Salamek
Copy link

Salamek commented Mar 30, 2023

@auvipy looking on the src, looks like this package attempts to build a CRON expression from celery.schedules.crontab object (or its values saved in database) and fails doing so when non standard values are used when constructing celery.schedules.crontab (like full name days or months are not supported in valid CRON expression, but celery.schedules.crontab accepts and uses them anyway) so someone should create def celery_crontab_to_cron(celery.schedules.crontab: celery_crontab) -> str: function that returns normalized valid CRON from celery.schedules.crontab and passes it into cron_descriptor ;-)

@auvipy auvipy merged commit 2798e36 into celery:main Apr 8, 2023
4 of 6 checks passed
@nijel nijel deleted the parse-error branch April 8, 2023 17:07
@schinckel
Copy link

For what it's worth, I was getting this error in the admin.

I was able to resolve it without new code by updating the crontab data to change wednesday to wed (for example).

@auvipy
Copy link
Member

auvipy commented Apr 26, 2023

For what it's worth, I was getting this error in the admin.

I was able to resolve it without new code by updating the crontab data to change wednesday to wed (for example).

should we mention this somewhere n docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants