Skip to content

Commit

Permalink
Don't permanently add zip DAGs to sys.path (#18384)
Browse files Browse the repository at this point in the history
Only leave the zip on `sys.path` while parsing that zip file, as we
shouldn't assume parsing only happens in separate processes. For
example, `resetdb` is used to run migrations, some of which parse
the DAGs.
  • Loading branch information
jedcunningham authored Sep 20, 2021
1 parent d1f3d8e commit a01c08b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 0 deletions.
3 changes: 3 additions & 0 deletions airflow/models/dagbag.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ def _load_modules_from_zip(self, filepath, safe_mode):
)
else:
self.import_errors[fileloc] = str(e)
finally:
if sys.path[0] == filepath:
del sys.path[0]
return mods

def _process_modules(self, filepath, mods, file_last_changed_on_disk):
Expand Down
4 changes: 4 additions & 0 deletions tests/models/test_dagbag.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
import logging
import os
import shutil
import sys
import textwrap
from copy import deepcopy
from datetime import datetime, timedelta, timezone
from tempfile import NamedTemporaryFile, mkdtemp
from unittest import mock
Expand Down Expand Up @@ -191,9 +193,11 @@ def test_zip(self):
"""
test the loading of a DAG within a zip file that includes dependencies
"""
syspath_before = deepcopy(sys.path)
dagbag = models.DagBag(dag_folder=self.empty_dir, include_examples=False)
dagbag.process_file(os.path.join(TEST_DAGS_FOLDER, "test_zip.zip"))
assert dagbag.get_dag("test_zip_dag")
assert sys.path == syspath_before # sys.path doesn't change

def test_process_file_cron_validity_check(self):
"""
Expand Down

0 comments on commit a01c08b

Please sign in to comment.