ianbuss commented on code in PR #23535:
URL: https://github.com/apache/airflow/pull/23535#discussion_r875841989
##########
airflow/utils/file.py:
##########
@@ -57,19 +60,18 @@ class _RegexpIgnoreRule(NamedTuple):
def compile(pattern: str, base_dir: Path, definition_file: Path) ->
Optional[_IgnoreRule]:
"""Build an ignore rule from the supplied regexp pattern and log a
useful warning if it is invalid"""
try:
- return _RegexpIgnoreRule(re.compile(pattern), base_dir.resolve())
+ return _RegexpIgnoreRule(re.compile(pattern), base_dir)
except re.error as e:
log.warning("Ignoring invalid regex '%s' from %s: %s", pattern,
definition_file, e)
return None
@staticmethod
def match(path: Path, rules: List[_IgnoreRule]) -> bool:
"""Match a list of ignore rules against the supplied path"""
- test_path: Path = path.resolve()
for rule in rules:
if not isinstance(rule, _RegexpIgnoreRule):
raise ValueError(f"_RegexpIgnoreRule cannot match rules of
type: {type(rule)}")
- if rule.pattern.search(str(test_path.relative_to(rule.base_dir)))
is not None:
+ if rule.pattern.search(str(path.relative_to(rule.base_dir))) is
not None:
Review Comment:
> In `def _find_path_from_directory(`
>
> ```
> for file in files:
> if file == ignore_file_name:
> continue
> abs_file_path = Path(root) / file
> if ignore_rule_type.match(abs_file_path, patterns):
> continue
> yield str(abs_file_path)
> ```
>
> I think (as a defensive programming method), I'd call .resolve() also here:
>
> ```
> abs_file_path = Path(root).resolve() / file
> ```
>
> Also it would be great to refactor the `path` file name to "abs_path" and
make sure everywhere it is used to be sure that we are using abs paths where we
intend to.
Yes, the `match` method of the `IgnoreRule` implementations is called in the
snippet @potiuk shared @uranusjr. I would also like to be defensive @potiuk and
that is essentially what we had before this change. Unfortunately though, we
cannot call `resolve` as it will normalise-away the symlink elements in the
path, which will result in user's patterns not matching when they should.
--
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]
