diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e64c31465edd1..4644bf195abb5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -506,6 +506,12 @@ repos: (?x) ^airflow/providers/.*\.py$ exclude: ^airflow/_vendor/ + - id: check-decorated-operator-implements-custom-name + name: Check @task decorator implements custom_operator_name + language: python + entry: ./scripts/ci/pre_commit/pre_commit_decorator_operator_implements_custom_name.py + pass_filenames: true + files: ^airflow/.*\.py$ - id: check-provide-create-sessions-imports language: pygrep name: Check provide_session and create_session imports diff --git a/STATIC_CODE_CHECKS.rst b/STATIC_CODE_CHECKS.rst index ca386413b32f9..fb6b92be86ae0 100644 --- a/STATIC_CODE_CHECKS.rst +++ b/STATIC_CODE_CHECKS.rst @@ -157,6 +157,8 @@ require Breeze Docker image to be build locally. +--------------------------------------------------------+------------------------------------------------------------------+---------+ | check-daysago-import-from-utils | Make sure days_ago is imported from airflow.utils.dates | | +--------------------------------------------------------+------------------------------------------------------------------+---------+ +| check-decorated-operator-implements-custom-name | Check @task decorator implements custom_operator_name | | ++--------------------------------------------------------+------------------------------------------------------------------+---------+ | check-docstring-param-types | Check that docstrings do not specify param types | | +--------------------------------------------------------+------------------------------------------------------------------+---------+ | check-example-dags-urls | Check that example dags url include provider versions | | diff --git a/airflow/providers/docker/decorators/docker.py b/airflow/providers/docker/decorators/docker.py index b07bb6cdff59d..bfd2b4db5d31f 100644 --- a/airflow/providers/docker/decorators/docker.py +++ b/airflow/providers/docker/decorators/docker.py @@ -62,6 +62,8 @@ class _DockerDecoratedOperator(DecoratedOperator, DockerOperator): Defaults to False. """ + custom_operator_name = "@task.docker" + template_fields: Sequence[str] = ('op_args', 'op_kwargs') # since we won't mutate the arguments, we should just do the shallow copy diff --git a/dev/breeze/src/airflow_breeze/pre_commit_ids.py b/dev/breeze/src/airflow_breeze/pre_commit_ids.py index a0a0970cb73ab..ac7d2ca684b80 100644 --- a/dev/breeze/src/airflow_breeze/pre_commit_ids.py +++ b/dev/breeze/src/airflow_breeze/pre_commit_ids.py @@ -35,6 +35,7 @@ 'check-builtin-literals', 'check-changelog-has-no-duplicates', 'check-daysago-import-from-utils', + 'check-decorated-operator-implements-custom-name', 'check-docstring-param-types', 'check-example-dags-urls', 'check-executables-have-shebangs', diff --git a/docs/apache-airflow/howto/create-custom-decorator.rst b/docs/apache-airflow/howto/create-custom-decorator.rst index 72c5bad658a3e..66d2c4a07fa30 100644 --- a/docs/apache-airflow/howto/create-custom-decorator.rst +++ b/docs/apache-airflow/howto/create-custom-decorator.rst @@ -31,6 +31,10 @@ tasks. The steps to create and register ``@task.foo`` are: ``airflow.decorators.base.DecoratedOperator``, Airflow will supply much of the needed functionality required to treat your new class as a taskflow native class. + You should also override the ``custom_operator_name`` attribute to provide a custom name for the task. For + example, ``_DockerDecoratedOperator`` in the ``apache-airflow-providers-docker`` provider sets this to + ``@task.docker`` to indicate the decorator name it implements. + 2. Create a ``foo_task`` function Once you have your decorated class, create a function like this, to convert diff --git a/images/breeze/output-commands-hash.txt b/images/breeze/output-commands-hash.txt index af6780245b408..8af15a6422dcc 100644 --- a/images/breeze/output-commands-hash.txt +++ b/images/breeze/output-commands-hash.txt @@ -54,7 +54,7 @@ setup:params:bc6b726d05240c22ac8522edc4a200ee setup:short_help:37a6259cc0c1dae299a7866489dff0bd shell:7baeb98c0103c594502ccf3d32ae24c6 start-airflow:3e793b11dc2158c54bfc189bfe20d6f2 -static-checks:03e96245e60c225ed94514ad7b42ceb0 +static-checks:a86b25f9281c47abf362a221b316c7a2 stop:8ebd8a42f1003495d37b884de5ac7ce6 testing:chain:68934a3e9455fa72420237eb05902327 testing:commands:c56d4fed05849ebfca609bd725200582 diff --git a/images/breeze/output_static-checks.svg b/images/breeze/output_static-checks.svg index b9f4790fd2e5c..da385d5682bd9 100644 --- a/images/breeze/output_static-checks.svg +++ b/images/breeze/output_static-checks.svg @@ -1,4 +1,4 @@ - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + + + + - Command: static-checks + Command: static-checks - + - - -Usage: breeze static-checks [OPTIONS] [PRECOMMIT_ARGS]... - -Run static checks. - -╭─ Pre-commit flags ───────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--type-tType(s) of the static checks to run (multiple can be added).                            │ -│(all | black | blacken-docs | check-airflow-2-2-compatibility |                         │ -│check-airflow-config-yaml-consistent | check-apache-license-rat |                       │ -│check-base-operator-partial-arguments | check-base-operator-usage |                     │ -│check-boring-cyborg-configuration | check-breeze-top-dependencies-limited |             │ -│check-builtin-literals | check-changelog-has-no-duplicates |                            │ -│check-daysago-import-from-utils | check-docstring-param-types | check-example-dags-urls │ -│| check-executables-have-shebangs | check-extra-packages-references | check-extras-order│ -│| check-for-inclusive-language | check-hooks-apply | check-incorrect-use-of-LoggingMixin│ -│| check-integrations-are-consistent | check-lazy-logging | check-merge-conflict |       │ -│check-newsfragments-are-valid | check-no-providers-in-core-examples |                   │ -│check-no-relative-imports | check-persist-credentials-disabled-in-github-workflows |    │ -│check-pre-commit-information-consistent | check-provide-create-sessions-imports |       │ -│check-provider-yaml-valid | check-providers-init-file-missing |                         │ -│check-providers-subpackages-init-file-exist | check-pydevd-left-in-code |               │ -│check-revision-heads-map | check-safe-filter-usage-in-html | check-setup-order |        │ -│check-start-date-not-used-in-defaults | check-system-tests-present |                    │ -│check-system-tests-tocs | check-xml | codespell | compile-www-assets |                  │ -│compile-www-assets-dev | create-missing-init-py-files-tests | debug-statements |        │ -│detect-private-key | doctoc | end-of-file-fixer | fix-encoding-pragma | flynt | identity│ -│| insert-license | isort | lint-chart-schema | lint-css | lint-dockerfile |             │ -│lint-helm-chart | lint-javascript | lint-json-schema | lint-markdown | lint-openapi |   │ -│mixed-line-ending | pretty-format-json | pydocstyle | python-no-log-warn | pyupgrade |  │ -│replace-bad-characters | rst-backticks | run-flake8 | run-mypy | run-shellcheck |       │ -│static-check-autoflake | trailing-whitespace | ts-compile-and-lint-javascript |         │ -│update-breeze-cmd-output | update-breeze-readme-config-hash | update-extras |           │ -│update-in-the-wild-to-be-sorted | update-inlined-dockerfile-scripts |                   │ -│update-local-yml-file | update-migration-references | update-providers-dependencies |   │ -│update-setup-cfg-file | update-spelling-wordlist-to-be-sorted |                         │ -│update-supported-versions | update-vendored-in-k8s-json-schema | update-version |       │ -│yamllint | yesqa)                                                                       │ -│--file-fList of files to run the checks on.(PATH)│ -│--all-files-aRun checks on all files.│ -│--show-diff-on-failure-sShow diff for files modified by the checks.│ -│--last-commit-cRun checks for all files in last commit. Mutually exclusive with --commit-ref.│ -│--commit-ref-rRun checks for this commit reference only (can be any git commit-ish reference).        │ -│Mutually exclusive with --last-commit.                                                  │ -│(TEXT)                                                                                  │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--verbose-vPrint verbose information about performed steps.│ -│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ -│--github-repository-gGitHub repository used to pull, push run images.(TEXT)[default: apache/airflow]│ -│--help-hShow this message and exit.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ + + +Usage: breeze static-checks [OPTIONS] [PRECOMMIT_ARGS]... + +Run static checks. + +╭─ Pre-commit flags ───────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--type-tType(s) of the static checks to run (multiple can be added).                            │ +│(all | black | blacken-docs | check-airflow-2-2-compatibility |                         │ +│check-airflow-config-yaml-consistent | check-apache-license-rat |                       │ +│check-base-operator-partial-arguments | check-base-operator-usage |                     │ +│check-boring-cyborg-configuration | check-breeze-top-dependencies-limited |             │ +│check-builtin-literals | check-changelog-has-no-duplicates |                            │ +│check-daysago-import-from-utils | check-decorated-operator-implements-custom-name |     │ +│check-docstring-param-types | check-example-dags-urls | check-executables-have-shebangs │ +│| check-extra-packages-references | check-extras-order | check-for-inclusive-language | │ +│check-hooks-apply | check-incorrect-use-of-LoggingMixin |                               │ +│check-integrations-are-consistent | check-lazy-logging | check-merge-conflict |         │ +│check-newsfragments-are-valid | check-no-providers-in-core-examples |                   │ +│check-no-relative-imports | check-persist-credentials-disabled-in-github-workflows |    │ +│check-pre-commit-information-consistent | check-provide-create-sessions-imports |       │ +│check-provider-yaml-valid | check-providers-init-file-missing |                         │ +│check-providers-subpackages-init-file-exist | check-pydevd-left-in-code |               │ +│check-revision-heads-map | check-safe-filter-usage-in-html | check-setup-order |        │ +│check-start-date-not-used-in-defaults | check-system-tests-present |                    │ +│check-system-tests-tocs | check-xml | codespell | compile-www-assets |                  │ +│compile-www-assets-dev | create-missing-init-py-files-tests | debug-statements |        │ +│detect-private-key | doctoc | end-of-file-fixer | fix-encoding-pragma | flynt | identity│ +│| insert-license | isort | lint-chart-schema | lint-css | lint-dockerfile |             │ +│lint-helm-chart | lint-javascript | lint-json-schema | lint-markdown | lint-openapi |   │ +│mixed-line-ending | pretty-format-json | pydocstyle | python-no-log-warn | pyupgrade |  │ +│replace-bad-characters | rst-backticks | run-flake8 | run-mypy | run-shellcheck |       │ +│static-check-autoflake | trailing-whitespace | ts-compile-and-lint-javascript |         │ +│update-breeze-cmd-output | update-breeze-readme-config-hash | update-extras |           │ +│update-in-the-wild-to-be-sorted | update-inlined-dockerfile-scripts |                   │ +│update-local-yml-file | update-migration-references | update-providers-dependencies |   │ +│update-setup-cfg-file | update-spelling-wordlist-to-be-sorted |                         │ +│update-supported-versions | update-vendored-in-k8s-json-schema | update-version |       │ +│yamllint | yesqa)                                                                       │ +│--file-fList of files to run the checks on.(PATH)│ +│--all-files-aRun checks on all files.│ +│--show-diff-on-failure-sShow diff for files modified by the checks.│ +│--last-commit-cRun checks for all files in last commit. Mutually exclusive with --commit-ref.│ +│--commit-ref-rRun checks for this commit reference only (can be any git commit-ish reference).        │ +│Mutually exclusive with --last-commit.                                                  │ +│(TEXT)                                                                                  │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--verbose-vPrint verbose information about performed steps.│ +│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ +│--github-repository-gGitHub repository used to pull, push run images.(TEXT)[default: apache/airflow]│ +│--help-hShow this message and exit.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ diff --git a/scripts/ci/pre_commit/pre_commit_decorator_operator_implements_custom_name.py b/scripts/ci/pre_commit/pre_commit_decorator_operator_implements_custom_name.py new file mode 100755 index 0000000000000..975d2f3473223 --- /dev/null +++ b/scripts/ci/pre_commit/pre_commit_decorator_operator_implements_custom_name.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +import ast +import itertools +import pathlib +import sys +from typing import Iterator + + +def iter_decorated_operators(source: pathlib.Path) -> Iterator[ast.ClassDef]: + mod = ast.parse(source.read_text("utf-8"), str(source)) + for node in ast.walk(mod): + if not isinstance(node, ast.ClassDef): + continue + if not any(isinstance(base, ast.Name) and base.id == "DecoratedOperator" for base in node.bases): + continue # Not a decorated operator. + yield node + + +def check_missing_custom_operator_name(klass: ast.ClassDef) -> bool: + for node in ast.iter_child_nodes(klass): + if isinstance(node, ast.AnnAssign): + if isinstance(node.target, ast.Name) and node.target.id == "custom_operator_name": + return True + elif isinstance(node, ast.Assign): + if any(isinstance(t, ast.Name) and t.id == "custom_operator_name" for t in node.targets): + return True + return False + + +def main(*args: str) -> int: + classes = itertools.chain.from_iterable(iter_decorated_operators(pathlib.Path(a)) for a in args[1:]) + results = ((k.name, check_missing_custom_operator_name(k)) for k in classes) + failures = [name for name, success in results if not success] + for failure in failures: + print(f"Missing custom_operator_name in class: {failure}") + return len(failures) + + +if __name__ == "__main__": + sys.exit(main(*sys.argv))