lzdanski commented on code in PR #38505:
URL: https://github.com/apache/airflow/pull/38505#discussion_r1548039059


##########
docs/apache-airflow/authoring-and-scheduling/timetable.rst:
##########
@@ -216,46 +215,47 @@ Timetables comparisons
 Differences between the two cron timetables
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-There are two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accepts a cron expression.
-There are some differences between the two:
-- `CronTriggerTimetable`_ does not take care of *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
-- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
intuitive and more similar to what people
-expect cron to behave than that of `CronDataIntervalTimetable`_ (when 
``catchup`` is ``False``).
+Airflow has two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accept a cron expression.
+However, there are differences between the two:
+- `CronTriggerTimetable`_ does not address *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
+- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
similar to how people
+expect cron to behave compared to `CronDataIntervalTimetable`_ when 
``catchup`` is ``False``.

Review Comment:
   ```suggestion
   Airflow has two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accept a cron expression.
   However, there are differences between the two:
   - `CronTriggerTimetable`_ does not address *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
   - The timestamp in the ``run_id``, the ``logical_date`` for 
`CronTriggerTimetable`_ and `CronDataIntervalTimetable`_  are defined 
differently based on how they handle the data interval.
   ```



##########
docs/apache-airflow/authoring-and-scheduling/timetable.rst:
##########
@@ -216,46 +215,47 @@ Timetables comparisons
 Differences between the two cron timetables
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-There are two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accepts a cron expression.
-There are some differences between the two:
-- `CronTriggerTimetable`_ does not take care of *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
-- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
intuitive and more similar to what people
-expect cron to behave than that of `CronDataIntervalTimetable`_ (when 
``catchup`` is ``False``).
+Airflow has two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accept a cron expression.
+However, there are differences between the two:
+- `CronTriggerTimetable`_ does not address *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
+- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
similar to how people
+expect cron to behave compared to `CronDataIntervalTimetable`_ when 
``catchup`` is ``False``.
 
 Whether taking care of *Data Interval*
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-`CronTriggerTimetable`_ *does not* care the idea of *data interval*. It means 
the value of ``data_interval_start``,
-``data_interval_end`` and legacy ``execution_date`` are the same - the time 
when a DAG run is triggered.
+`CronTriggerTimetable`_ *does not* include *data interval*. This means that 
the value of ``data_interval_start`` and
+``data_interval_end`` (and the legacy ``execution_date``) are the same; the 
time when a DAG run is triggered.
 
-On the other hand, `CronDataIntervalTimetable`_ *does* care the idea of *data 
interval*. It means the value of
-``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. They are the start
-and end of the interval respectively.
+However, `CronDataIntervalTimetable`_ *does* include *data interval*. This 
means the value of
+``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. ``data_interval_start`` is the time when a
+DAG run is triggered and ``data_interval_end`` is the end of the interval.
 
 The time when a DAG run is triggered
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-There is no difference between the two when ``catchup`` is ``True``. 
:ref:`dag-catchup` tells you how DAG runs are
+There is no difference between `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ when ``catchup`` is ``True``. :ref:`dag-catchup` 
tells you how DAG runs are
 triggered when ``catchup`` is ``True``.
 
-When ``catchup`` is ``False``, there is difference in how a new DAG run is 
triggered. `CronTriggerTimetable`_ triggers
-a new DAG run *after* the current time, while `CronDataIntervalTimetable`_ 
does *before* the current time (assuming
-the value of ``start_date`` is past time).
+When ``catchup`` is ``False``, there is difference in how a new DAG run is 
triggered.

Review Comment:
   ```suggestion
   ```



##########
docs/apache-airflow/authoring-and-scheduling/timetable.rst:
##########
@@ -216,46 +215,47 @@ Timetables comparisons
 Differences between the two cron timetables
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-There are two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accepts a cron expression.
-There are some differences between the two:
-- `CronTriggerTimetable`_ does not take care of *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
-- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
intuitive and more similar to what people
-expect cron to behave than that of `CronDataIntervalTimetable`_ (when 
``catchup`` is ``False``).
+Airflow has two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accept a cron expression.
+However, there are differences between the two:
+- `CronTriggerTimetable`_ does not address *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
+- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
similar to how people
+expect cron to behave compared to `CronDataIntervalTimetable`_ when 
``catchup`` is ``False``.
 
 Whether taking care of *Data Interval*
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-`CronTriggerTimetable`_ *does not* care the idea of *data interval*. It means 
the value of ``data_interval_start``,
-``data_interval_end`` and legacy ``execution_date`` are the same - the time 
when a DAG run is triggered.
+`CronTriggerTimetable`_ *does not* include *data interval*. This means that 
the value of ``data_interval_start`` and
+``data_interval_end`` (and the legacy ``execution_date``) are the same; the 
time when a DAG run is triggered.
 
-On the other hand, `CronDataIntervalTimetable`_ *does* care the idea of *data 
interval*. It means the value of
-``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. They are the start
-and end of the interval respectively.
+However, `CronDataIntervalTimetable`_ *does* include *data interval*. This 
means the value of
+``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. ``data_interval_start`` is the time when a
+DAG run is triggered and ``data_interval_end`` is the end of the interval.
 
 The time when a DAG run is triggered
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-There is no difference between the two when ``catchup`` is ``True``. 
:ref:`dag-catchup` tells you how DAG runs are
+There is no difference between `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ when ``catchup`` is ``True``. :ref:`dag-catchup` 
tells you how DAG runs are
 triggered when ``catchup`` is ``True``.

Review Comment:
   Add new subsection - catchup = false
   When you first make a brand new dag, if you set the start date in the past 
and catchup=true, then it'll run all the DAGs that had been previously.
   If you pause a dag, this is when catchup matters. Because if your DAG is 
paused for a month, you can specify whether or not the DAG needs to run for all 
the days that had been missed. 



##########
docs/apache-airflow/authoring-and-scheduling/timetable.rst:
##########
@@ -216,46 +215,47 @@ Timetables comparisons
 Differences between the two cron timetables
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-There are two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accepts a cron expression.
-There are some differences between the two:
-- `CronTriggerTimetable`_ does not take care of *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
-- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
intuitive and more similar to what people
-expect cron to behave than that of `CronDataIntervalTimetable`_ (when 
``catchup`` is ``False``).
+Airflow has two timetables `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ that accept a cron expression.
+However, there are differences between the two:
+- `CronTriggerTimetable`_ does not address *Data Interval*, while 
`CronDataIntervalTimetable`_ does.
+- The time when a DAG run is triggered by `CronTriggerTimetable`_ is more 
similar to how people
+expect cron to behave compared to `CronDataIntervalTimetable`_ when 
``catchup`` is ``False``.
 
 Whether taking care of *Data Interval*
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-`CronTriggerTimetable`_ *does not* care the idea of *data interval*. It means 
the value of ``data_interval_start``,
-``data_interval_end`` and legacy ``execution_date`` are the same - the time 
when a DAG run is triggered.
+`CronTriggerTimetable`_ *does not* include *data interval*. This means that 
the value of ``data_interval_start`` and
+``data_interval_end`` (and the legacy ``execution_date``) are the same; the 
time when a DAG run is triggered.
 
-On the other hand, `CronDataIntervalTimetable`_ *does* care the idea of *data 
interval*. It means the value of
-``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. They are the start
-and end of the interval respectively.
+However, `CronDataIntervalTimetable`_ *does* include *data interval*. This 
means the value of
+``data_interval_start`` and ``data_interval_end`` (and legacy 
``execution_date``) are different. ``data_interval_start`` is the time when a
+DAG run is triggered and ``data_interval_end`` is the end of the interval.
 
 The time when a DAG run is triggered
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-There is no difference between the two when ``catchup`` is ``True``. 
:ref:`dag-catchup` tells you how DAG runs are
+There is no difference between `CronTriggerTimetable`_ and 
`CronDataIntervalTimetable`_ when ``catchup`` is ``True``. :ref:`dag-catchup` 
tells you how DAG runs are
 triggered when ``catchup`` is ``True``.
 
-When ``catchup`` is ``False``, there is difference in how a new DAG run is 
triggered. `CronTriggerTimetable`_ triggers
-a new DAG run *after* the current time, while `CronDataIntervalTimetable`_ 
does *before* the current time (assuming
-the value of ``start_date`` is past time).
+When ``catchup`` is ``False``, there is difference in how a new DAG run is 
triggered.
 
-Here is an example showing how the first DAG run is triggered. Supposes there 
is a cron expression ``@daily`` or
-``0 0 * * *``, which is aimed to run at 12AM every day. If you enable DAGs 
using the two timetables at 3PM on January
-31st, `CronTriggerTimetable`_ will trigger a new DAG run at 12AM on February 
1st. `CronDataIntervalTimetable`_, on the other
-hand, will immediately trigger a new DAG run which is supposed to trigger at 
12AM on January 31st if the DAG had been
-enabled beforehand.
+`CronTriggerTimetable`_ and `CronDataIntervalTimetable`_ trigger DAG runs at 
the same time. However, the timestamp for the ``run_id`` is different for each.

Review Comment:
   ```suggestion
   `CronTriggerTimetable`_ and `CronDataIntervalTimetable`_ trigger DAG runs at 
the same time. However, the timestamp for the ``run_id`` (``logical_date``) is 
different for each.
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to