Support glob syntax in .airflowignore files (#21392) #22051

ianbuss · 2022-03-07T15:13:18Z

A new configuration parameter "CORE_IGNOREFILE_SYNTAX" is added to
allow patterns in .airflowignore files to be interpreted as either
regular expressions (the default) or glob expressions as found in
.gitignore files. This allows users to use patterns they will be
familiar with from tools such as git, helm and docker.

Glob expressions support wildcard matches ("*", "?") within a directory
as well as character classes ("[0-9]"). In addition, zero or more
directories can be matched using "**". Patterns can be negated by
prefixing a "!" at the beginning of the pattern.

The "fnmatch" library in core Python does not produce patterns that are
fully compliant with the kind of patterns that users will be used to
from gitignore or dockerignore files, so the globs are parsed using
the gitignore-parser package from PyPI.

To aid with debugging ignorefile patterns a more helpful error
message is emitted in the logs for invalid patterns, which are
now skipped rather than causing a hard-to-read scheduler stack trace.

closes: #21392

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

boring-cyborg · 2022-03-07T15:13:21Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: [email protected]
Slack: https://s.apache.org/airflow-slack

airflow/config_templates/config.yml

mik-laj · 2022-03-07T15:34:03Z

airflow/utils/file.py

+        elif '**' in pattern and '/**/' not in pattern:
+            reason = "'**' must be between '/' chars when not at beginning or end of pattern"
+        if reason:
+            log.warning(f"Ignoring glob '{pattern}' from {ignore_file_path}: {reason}")


Suggested change

log.warning(f"Ignoring glob '{pattern}' from {ignore_file_path}: {reason}")

log.warning(f"Ignoring glob '%s' from %s: %s:', pattern, ignore_file_path, reason)

Please avoid stirring formatting before passing to logger.

ianbuss · 2022-03-11T10:51:04Z

Thanks for your review @mik-laj! I will make the changes to the PR based on your comments. I've also found some logic bugs in the dependency being used for the gitignore parsing, so currently looking into workarounds and/or alternatives.

github-actions · 2022-03-21T11:21:42Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

potiuk · 2022-03-21T11:21:46Z

Seme doc building errors and tests failing @ianbuss :)

A new configuration parameter "CORE_IGNORE_FILE_SYNTAX" is added to allow patterns in .airflowignore files to be interpreted as either regular expressions (the default) or glob expressions as found in .gitignore files. This allows users to use patterns they will be familiar with from tools such as git, helm and docker. Glob expressions support wildcard matches ("*", "?") within a directory as well as character classes ("[0-9]"). In addition, zero or more directories can be matched using "**". Patterns can be negated by prefixing a "!" at the beginning of the pattern. The "fnmatch" library in core Python does not produce patterns that are fully compliant with the kind of patterns that users will be used to from gitignore or dockerignore files, so the globs are parsed using the pathspec package from PyPI. To aid with debugging ignorefile patterns a more helpful error message is emitted in the logs for invalid patterns, which are now skipped rather than causing a hard-to-read scheduler stack trace.

ianbuss · 2022-04-12T14:19:31Z

Thanks @potiuk looking into the failing tests now!

uranusjr

Some nitpicks

uranusjr · 2022-04-12T20:10:23Z

airflow/models/dagbag.py

-        Note that if a ``.airflowignore`` file is found while processing
+        Note that if an ``.airflowignore`` file is found while processing


For me a is right because .airflowignore is prodounced dot airflow ignore.

Hmm, yeah, I can go long with that :) Will update.

uranusjr · 2022-04-12T20:10:45Z

airflow/config_templates/config.yml

+        The pattern syntax used in the ".airflowignore" files in the DAG directories. Valid values are
+        ``regexp`` or ``glob``.


Suggested change

The pattern syntax used in the ".airflowignore" files in the DAG directories. Valid values are

``regexp`` or ``glob``.

The pattern syntax used in the ".airflowignore" files in the DAG directories. Valid values are

``regexp`` and ``glob``.

I think this is correct English? (I’m non-native so don’t take my words)

I think either can work actually in this context, I chose "or" since you have to choose one or the other.

Either makes sense for me too

airflow/models/dagbag.py

airflow/utils/file.py

ianbuss · 2022-04-12T21:15:14Z

Thanks for your additional review @uranusjr have made changes to the docs.

boring-cyborg · 2022-04-13T10:20:01Z

Awesome work, congrats on your first merged pull request!

potiuk · 2022-04-13T10:23:40Z

Cool it made it to 2.3.0 :)

ianbuss · 2022-04-13T10:28:50Z

w00t, thanks @kaxil @potiuk @uranusjr

jedcunningham · 2022-04-14T00:13:06Z

Congrats on your first commit @ianbuss 🎉

ianbuss requested review from kaxil and potiuk as code owners March 7, 2022 15:13

boring-cyborg bot added area:plugins kind:documentation labels Mar 7, 2022

mik-laj reviewed Mar 7, 2022

View reviewed changes

airflow/config_templates/config.yml Show resolved Hide resolved

mik-laj reviewed Mar 7, 2022

View reviewed changes

ianbuss force-pushed the issue-21392-airflowignore branch from 9f021ad to cf23b3a Compare March 16, 2022 20:50

ianbuss requested review from XD-DENG and ashb as code owners March 16, 2022 20:50

ianbuss requested a review from mik-laj March 16, 2022 21:36

potiuk approved these changes Mar 21, 2022

View reviewed changes

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 21, 2022

Fix some UT and docs build issues

9e7b4b8

ianbuss force-pushed the issue-21392-airflowignore branch from cf23b3a to 9e7b4b8 Compare April 12, 2022 17:54

uranusjr reviewed Apr 12, 2022

View reviewed changes

airflow/utils/file.py Outdated Show resolved Hide resolved

uranusjr reviewed Apr 12, 2022

View reviewed changes

airflow/utils/file.py Outdated Show resolved Hide resolved

Some docs review feedback

ff88754

kaxil approved these changes Apr 13, 2022

View reviewed changes

kaxil merged commit 0cd8833 into apache:main Apr 13, 2022

uranusjr approved these changes Apr 13, 2022

View reviewed changes

jedcunningham added the type:improvement Changelog: Improvements label Apr 14, 2022

tirkarthi mentioned this pull request May 6, 2022

Airflow .airflowignore not handling soft link properly. #23532

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support glob syntax in .airflowignore files (#21392) #22051

Support glob syntax in .airflowignore files (#21392) #22051

ianbuss commented Mar 7, 2022 •

edited by eladkal

Loading

boring-cyborg bot commented Mar 7, 2022

mik-laj Mar 7, 2022

ianbuss commented Mar 11, 2022

github-actions bot commented Mar 21, 2022

potiuk commented Mar 21, 2022

ianbuss commented Apr 12, 2022

uranusjr left a comment

uranusjr Apr 12, 2022

ianbuss Apr 12, 2022

uranusjr Apr 12, 2022

ianbuss Apr 12, 2022

kaxil Apr 13, 2022

ianbuss commented Apr 12, 2022

boring-cyborg bot commented Apr 13, 2022

potiuk commented Apr 13, 2022

ianbuss commented Apr 13, 2022

jedcunningham commented Apr 14, 2022

	log.warning(f"Ignoring glob '{pattern}' from {ignore_file_path}: {reason}")
	log.warning(f"Ignoring glob '%s' from %s: %s:', pattern, ignore_file_path, reason)

		Note that if a ``.airflowignore`` file is found while processing
		Note that if an ``.airflowignore`` file is found while processing

		The pattern syntax used in the ".airflowignore" files in the DAG directories. Valid values are
		``regexp`` or ``glob``.

Support glob syntax in .airflowignore files (#21392) #22051

Support glob syntax in .airflowignore files (#21392) #22051

Conversation

ianbuss commented Mar 7, 2022 • edited by eladkal Loading

closes: #21392

boring-cyborg bot commented Mar 7, 2022

mik-laj Mar 7, 2022

Choose a reason for hiding this comment

ianbuss commented Mar 11, 2022

github-actions bot commented Mar 21, 2022

potiuk commented Mar 21, 2022

ianbuss commented Apr 12, 2022

uranusjr left a comment

Choose a reason for hiding this comment

uranusjr Apr 12, 2022

Choose a reason for hiding this comment

ianbuss Apr 12, 2022

Choose a reason for hiding this comment

uranusjr Apr 12, 2022

Choose a reason for hiding this comment

ianbuss Apr 12, 2022

Choose a reason for hiding this comment

kaxil Apr 13, 2022

Choose a reason for hiding this comment

ianbuss commented Apr 12, 2022

boring-cyborg bot commented Apr 13, 2022

potiuk commented Apr 13, 2022

ianbuss commented Apr 13, 2022

jedcunningham commented Apr 14, 2022

ianbuss commented Mar 7, 2022 •

edited by eladkal

Loading