bingqin2 opened a new issue, #72982:
URL: https://github.com/apache/airflow/issues/72982

   ### Body
   
   While looking for bugs in provider code I wrote a small AST-based script 
that, for every operator/hook/trigger class, takes each `self.<attr> = 
<constructor parameter>` assignment in `__init__` and checks whether `.<attr>` 
is ever read anywhere in that provider's source. Attributes in 
`template_fields` and private attributes are skipped. The hits were checked by 
hand, because a parameter can legitimately be read by a base class 
(`HttpHook.auth_type`), by another provider 
(`DataflowConfiguration.check_if_running` is read by `apache.beam`) or by the 
Task SDK (`KubernetesPodOperator.durable`).
   
   What remains is a set of documented parameters that have no effect at all. 
Users set them and nothing happens, which is worse than not having the option.
   
   **Fixed so far (each with a regression test)**
   
   - [x] `HttpToS3Operator.log_response` (amazon) — #72951
   - [x] `HttpToGCSOperator.log_response` (google) — #72952
   - [x] `GCSTimeSpanFileTransformOperator.download_num_attempts` / 
`upload_num_attempts` (google) — #72953, a regression from #62196
   - [x] `idp_extra_params_dict` in the Google IdP flow (google) — #72954 
(stored but never passed to the token supplier; the connection form also saved 
the field under a different key than the hook read)
   - [x] `FABAuthManagerRoles.get_permissions` session handling (fab) — #72950 
(found by the same read-through, different bug class)
   
   **Still open, and the decision I would like a maintainer's view on**
   
   - [ ] `AzureBatchOperator.batch_max_retries` (microsoft.azure) — documented 
as "the number of times to retry this batch operation", never used. 
`azure-batch` 15 is `azure-core` based, so it can be wired through as 
`retry_total` on the `BatchClient` (or per call). I can take this one.
   - [ ] `SparkKubernetesOperator.success_run_history_limit` (cncf.kubernetes) 
— documented as "number of past successful runs of the application to keep", 
never used, and the corresponding CRD field (`successfulRunHistoryLimit`) only 
exists on `ScheduledSparkApplication`, not on the `SparkApplication` the 
operator submits. Deprecate?
   - [ ] `BigtableDeleteTableOperator.app_profile_id` (google) — documented, 
stored, but `BigtableHook.delete_table` has no such argument and the admin API 
does not take one. Deprecate?
   - [ ] `DataflowConfiguration.max_num_workers` (google) — documented as 
"maximum amount of workers", but neither the google nor the apache.beam 
provider reads it (the other `DataflowConfiguration` fields are read by the 
Beam operators). Wire it into the pipeline options, or deprecate?
   
   Two neighbouring findings from a second script that cross-checks the 
`status` values triggers emit against the values `execute_complete` tests for:
   
   - [ ] `EksDeleteClusterOperator.execute_complete` checks for `"success"` but 
`EksDeleteClusterTrigger` yields `"deleted"`, so the success log line is 
unreachable (cosmetic).
   - [ ] `DbtCloudJobRunSensor.execute_complete` raises on 
`"error"`/`"cancelled"` but returns normally on `"timeout"`, which 
`DbtCloudRunJobTrigger` can emit when constructed with `execution_deadline` 
(the sensor itself does not pass one today, so this is latent).
   
   Happy to open PRs for whichever direction maintainers prefer, and to share 
the scripts if a permanent check (with an allowlist, like the one in 
`test_project_structure.py`) seems worth it.
   
   ### Committer
   
   - [ ] I acknowledge that I am a maintainer/committer of the Apache Airflow 
project.
   


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