ianbuss commented on code in PR #23535:
URL: https://github.com/apache/airflow/pull/23535#discussion_r875844123
##########
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:
> I generally prefer to change all the paths to "absolute" as early as
possible and only use the absolute ones wherever I can (and derive relative
paths when needed, knowing that all the paths I might get are always absolute
(and be explicit about it).
>
> That enormously helps with lowering the mental effort needed to read and
understand what's going on - the "relative" paths require you to also know
(sometimes you have to know it out of a thin air) what they are relative to.
Couldn't agree more, although some extra complexity here is in play here due
to the use of relative patterns in the ignore files. I'll go over the code
again to see if there are opportunities to simplify further. I will definitely
change the variable names to `abs_path` here though to reduce the cognitive
load.
--
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]
