potiuk commented on a change in pull request #20848: URL: https://github.com/apache/airflow/pull/20848#discussion_r790590004
########## File path: scripts/ci/pre_commit/pre_commit_check_pre_commit_hook_names.py ########## @@ -50,16 +57,112 @@ def main() -> int: return retval -def get_errors(content, max_length): +def get_errors_and_ids(content, max_length): errors = [] + ids = set() for repo in content['repos']: for hook in repo['hooks']: + if 'id' in hook: + id = hook['id'] + ids.add(id) if 'name' not in hook: continue name = hook['name'] if len(name) > max_length: errors.append(name) - return errors + return errors, list(ids) + + +def render_template( + searchpath: Path, + template_name: str, + context: Dict[str, Any], + extension: str, + autoescape: bool = True, + keep_trailing_newline: bool = False, +) -> str: + """ + Renders template based on its name. Reads the template from <name>_TEMPLATE.md.jinja2 in current dir. + :param template_name: name of the template to use + :param context: Jinja2 context + :param extension: Target file extension + :param autoescape: Whether to autoescape HTML + :param keep_trailing_newline: Whether to keep the newline in rendered output + :return: rendered template + """ + import jinja2 + + template_loader = jinja2.FileSystemLoader(searchpath=searchpath) + template_env = jinja2.Environment( + loader=template_loader, + undefined=jinja2.StrictUndefined, + autoescape=autoescape, + keep_trailing_newline=keep_trailing_newline, + ) + template = template_env.get_template(f"{template_name}_TEMPLATE{extension}.jinja2") + content: str = template.render(context) + return content + + +def get_precommit_jinja_context(pre_commit_ids: List) -> Dict[str, List]: + context: Dict[str, List] = {"PRE_COMMIT_IDS": pre_commit_ids} + return context + + +def get_target_precommit_parent_dir(): + airflow_source = Path.cwd() + pre_commit_ids_file = Path(airflow_source, "dev", "breeze", "src", "airflow_breeze") + return pre_commit_ids_file + + +def get_breeze_pyproject_toml_dir(): + airflow_source = Path.cwd() + breeze_pyproject_toml_file = Path(airflow_source, "dev", "breeze") + return breeze_pyproject_toml_file + + +@lru_cache(maxsize=None) +def black_mode(): Review comment: I think it's really the matter of "is it actually executed with every PR". Pre-commits are executed always with every PR (nearly). So if something breaks, we will likely find out quickly. Unit tests are really there to execute the code often and assert if it still behaves as it should. Pre-comit is slighlty different because we do not 'formally" assert if they do what they should do - this is small risk, becuase the precommit might simply "do it wrongly". But that's an acceptable risk. Pre-commits are there to prevent us from making mistakes that we shoudl also caught during the code review (basically - pre-commits automate big part of the reviewer's job really), so we are likely to catch any of such mistakes during code review. -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org