Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect .airflowignore behavior with multiple nested directories #11994

Merged
merged 2 commits into from
Nov 2, 2020

Conversation

megaserg
Copy link
Contributor

@megaserg megaserg commented Oct 31, 2020

Problem:
In find_path_from_directory(), the patterns_by_dir map is incorrectly emptied and reset with patterns from the current directory, effectively forgetting about the patterns added earlier, for directories not yet processed. The modified testcase demonstrates the issue.

Solution:
Instead of resetting the map, update it with the new entries. An entry for a directory is added only once, namely when its immediate parent directory is processed, therefore we never overwrite any key in the map.

Note:
The test is currently marked as heisentest; PR #11993 unmarks it. I imagine that PR should go first, and this PR should be rebased after that.

@megaserg
Copy link
Contributor Author

@j-y-matsubara does this make sense?

@megaserg
Copy link
Contributor Author

megaserg commented Nov 2, 2020

@potiuk would you be so kind as to take a look at another bugfix in airflowignore? Thanks!

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 2, 2020
@github-actions
Copy link

github-actions bot commented Nov 2, 2020

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@potiuk
Copy link
Member

potiuk commented Nov 2, 2020

Some temporary blip when pulling image

@potiuk
Copy link
Member

potiuk commented Nov 2, 2020

No need to run full tests bot :)

@potiuk potiuk merged commit 5204ff6 into apache:master Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:plugins full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants