wfatih commented on issue #48448:
URL: https://github.com/apache/airflow/issues/48448#issuecomment-5748624566

   I would like to take this one, and I measured the drift first so the scope 
is based on numbers rather than guesses.
   
   A static scan — `ast` only, nothing imported — of 
`task-sdk/src/airflow/sdk/definitions/decorators/__init__.pyi` against the 
operator behind each `task-decorators` entry registered in `provider.yaml` 
finds:
   
   * **11 registered decorators with no stub entry at all**: `agent`, 
`analytics`, `llm`, `llm_branch`, `llm_file_analysis`, `llm_schema_compare`, 
`llm_sql`, `sftp_sensor`, `snowpark`, `sql`, `stub`. `@task.llm(...)` and 
friends fall back to `__getattr__` today, so they get no parameter checking at 
all.
   * **Parameter drift in 7 of the 13 decorators that do have a stub.** A few 
examples: `python` is missing `templates_exts`, which `PythonOperator.__init__` 
accepts; `virtualenv` and `external_python` are missing `string_args`, 
`templates_exts` and `expect_airflow`; `kubernetes` and `kubernetes_cmd` are 
each missing around thirteen `KubernetesPodOperator` parameters, including 
`cmds`, `callbacks`, `logging_interval` and `runtime_class_name`.
   
   Not every difference is drift. `use_dill`, `serializer` and `python_command` 
exist only on the decorator side, and `bash_command`, `command` and `arguments` 
are deliberately supplied by the decorated function rather than the caller, so 
the check needs an explicit exemption list instead of a strict equality 
assertion.
   
   On where the check should live: the earlier attempt in #51048 needed Airflow 
importable to compare signatures, which pushed it into an in-container script 
and is where it got stuck. Comparing the stub against the operator `__init__` 
can be done entirely statically, so it fits a plain `scripts/ci/prek/` hook 
with no Airflow dependency and no container, runnable on a clean checkout. I 
have a working prototype of that comparison, which is where the numbers above 
come from.
   
   Three questions before I write the real thing:
   
   1. Is a static prek hook the shape you want, or would you still rather have 
this in the test suite?
   2. Are the 11 missing decorators in scope here, or a follow-up? Adding them 
means writing stubs for provider decorators that never had one.
   3. For the drift the check finds, should the same PR fix the stubs — the 
kubernetes ones are the bulk of it — or should the check land with those 
exempted and the fixes follow per provider?
   
   ---
   Drafted-by: Claude Opus 5 (1M context); reviewed by @wfatih before posting
   


-- 
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]

Reply via email to