You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/06/30 14:01:19 UTC

[GitHub] [airflow] uranusjr opened a new issue #16731: Additional cron expression validation

uranusjr opened a new issue #16731:
URL: https://github.com/apache/airflow/issues/16731


   **Description**
   Airflow should outline (in documentation) what cron expressions are supported, and perform appropriate additional validation cron expressions.
   
   **Use case / motivation**
   #16692 proposed displaying a short description about the cron expression in the web UI. This bring out the issue that currently the cron expression formats supported by Airflow are essentially defined by croniter implementation, which is non-standard (POSIX only allows five-segment formats) and sometimes buggy (see https://github.com/apache/airflow/issues/16107#issuecomment-869559200). Since croniter itself does not offer expression description, we need to do this with another tool, meaning that non-standard expressions can be incorrectly described and cause user confusion.
   
   It would be beneficial for Airflow to explicitly specify what cron expression formats are supported (only POSIX, or POSIX plus extensions like the optional year segment, or something else), and perform additional validation to ensure croniter does not receive formats not supported by Airflow, even if it can handle them. Personally I’m inclined to only do POSIX and reject the sixth segment entirely, at least until croniter fixes its handling to correctly interpret it (see taichino/croniter#76).
   
   **Are you willing to submit a PR?**
   Yeah
   
   **Related Issues**
   https://github.com/apache/airflow/issues/16107#issuecomment-869559200
   #16692
   taichino/croniter#76


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] eladkal commented on issue #16731: Additional cron expression validation

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #16731:
URL: https://github.com/apache/airflow/issues/16731#issuecomment-871524976


   By addition validation do you mean to remove the 6 segmant and then trust croniter ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] uranusjr edited a comment on issue #16731: Additional cron expression validation

Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on issue #16731:
URL: https://github.com/apache/airflow/issues/16731#issuecomment-871556758


   I think we should build a simple validator to check for the whole string before passing it into croniter (and hopefully croniter will always work if the string passes our test).
   
   [The standard POSIX syntax](https://www.unix.com/man-page/posix/1p/crontab/) is fairly easy to parse, plus support for SUN-SAT. Is JAN-DEC a GNU extension? That can be added as well if it works in croniter.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] uranusjr commented on issue #16731: Additional cron expression validation

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #16731:
URL: https://github.com/apache/airflow/issues/16731#issuecomment-871556758


   I think we should build a simple validator to check for the whole string before passing it into croniter (and hopefully croniter will always work if the string passes our test).
   
   [The standard POSIX syntax](https://www.unix.com/man-page/posix/1p/crontab/) is fairly easy to parse, plus support to SUN-SAT. Is JAN-DEC a GNU extension? That can be added as well if it works in croniter.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on issue #16731: Additional cron expression validation

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #16731:
URL: https://github.com/apache/airflow/issues/16731#issuecomment-872162908


   We don't want to restrict to just POSIX cron -- some of the non-POSIX extensions are widely supported and useful `0 0 * * 5#3,L5` for instace.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org