Skip to content

Commit 37c0cb6

Browse files
authored
Add a note against use of top level code in timetable (#26649)
Accessing Variables, Connections at top level of code is bad practice Add a section in best practices ref for timetable
1 parent 26f94c5 commit 37c0cb6

File tree

4 files changed

+52
-3
lines changed

4 files changed

+52
-3
lines changed

airflow/serialization/serialized_objects.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ def __init__(self, type_string: str) -> None:
165165
self.type_string = type_string
166166

167167
def __str__(self) -> str:
168-
return f"Timetable class {self.type_string!r} is not registered"
168+
return (
169+
f"Timetable class {self.type_string!r} is not registered or "
170+
"you have a top level database access that disrupted the session. "
171+
"Please check the airflow best practices documentation."
172+
)
169173

170174

171175
def _encode_timetable(var: Timetable) -> dict[str, Any]:

docs/apache-airflow/best-practices.rst

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,41 @@ or if you need to deserialize a json object from the variable :
216216
For security purpose, you're recommended to use the :ref:`Secrets Backend<secrets_backend_configuration>`
217217
for any variable that contains sensitive data.
218218

219+
.. _best_practices/timetables:
220+
221+
Timetables
222+
----------
223+
Avoid using Airflow Variables/Connections or accessing airflow database at the top level of your timetable code.
224+
Database access should be delayed until the execution time of the DAG. This means that you should not have variables/connections retrieval
225+
as argument to your timetable class initialization or have Variable/connection at the top level of your custom timetable module.
226+
227+
Bad example:
228+
229+
.. code-block:: python
230+
231+
from airflow.models.variable import Variable
232+
from airflow.timetables.interval import CronDataIntervalTimetable
233+
234+
235+
class CustomTimetable(CronDataIntervalTimetable):
236+
def __init__(self, *args, something=Variable.get('something'), **kwargs):
237+
self._something = something
238+
super().__init__(*args, **kwargs)
239+
240+
Good example:
241+
242+
.. code-block:: python
243+
244+
from airflow.models.variable import Variable
245+
from airflow.timetables.interval import CronDataIntervalTimetable
246+
247+
248+
class CustomTimetable(CronDataIntervalTimetable):
249+
def __init__(self, *args, something='something', **kwargs):
250+
self._something = Variable.get(something)
251+
super().__init__(*args, **kwargs)
252+
253+
219254
Triggering DAGs after changes
220255
-----------------------------
221256

docs/apache-airflow/concepts/timetable.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ As such, Airflow allows for custom timetables to be written in plugins and used
5151
DAGs. An example demonstrating a custom timetable can be found in the
5252
:doc:`/howto/timetable` how-to guide.
5353

54+
.. note::
55+
56+
As a general rule, always access Variables, Connections etc or anything that would access
57+
the database as late as possible in your code. See :ref:`best_practices/timetables`
58+
for more best practices to follow.
59+
5460
Built-in Timetables
5561
-------------------
5662

tests/serialization/test_dag_serialization.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,9 @@ def test_dag_serialization_unregistered_custom_timetable(self):
406406
message = (
407407
"Failed to serialize DAG 'simple_dag': Timetable class "
408408
"'tests.test_utils.timetables.CustomSerializationTimetable' "
409-
"is not registered"
409+
"is not registered or "
410+
"you have a top level database access that disrupted the session. "
411+
"Please check the airflow best practices documentation."
410412
)
411413
assert str(ctx.value) == message
412414

@@ -712,7 +714,9 @@ def test_deserialization_timetable_unregistered(self):
712714
message = (
713715
"Timetable class "
714716
"'tests.test_utils.timetables.CustomSerializationTimetable' "
715-
"is not registered"
717+
"is not registered or "
718+
"you have a top level database access that disrupted the session. "
719+
"Please check the airflow best practices documentation."
716720
)
717721
assert str(ctx.value) == message
718722

0 commit comments

Comments
 (0)