Skip to content

Provider operator parameters that are documented but have no effect #72982

Open
Task
Open
Provider operator parameters that are documented but have no effect#72982
Task
Labels
area:providerskind:taskA task that needs to be completed as part of a larger issue

Description

@bingqin2
bingqin2
opened on Sep 11, 2026

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) (status as of 2026-10-05)

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). Taken by @AbarnaaSree: Support max retries in Azure Batch operator #73071.
  • 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? Taken by @garvit-arora: Wire DataflowConfiguration.max_num_workers into pipeline options #73922.

Two neighbouring findings from a second script that cross-checks the status values triggers emit against the values execute_complete tests for:

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.

Activity

  1. AbarnaaSree commented on Sep 12, 2026

    @AbarnaaSree
    AbarnaaSree
    on Sep 12, 2026

    Hi @bingqin2, I’d like to work on the AzureBatchOperator.batch_max_retries issue.

    The parameter is documented and stored but currently has no effect, and wiring it through to the Azure Batch client retry configuration seems like a well-scoped fix.

    I’ll first verify the current implementation and existing retry-related tests, then add regression coverage to ensure batch_max_retries is actually honored without changing the default behavior.

    If that direction is okay with the maintainers, I’d be happy to submit a PR.

  2. bingqin2 commented on Sep 13, 2026

    @bingqin2
    bingqin2
    on Sep 13, 2026
    ContributorAuthor

    Hi @AbarnaaSree, please go ahead, it is yours. A few pointers from what I checked while writing this up:

    • AzureBatchHook.__init__ only takes azure_batch_conn_id, and get_conn() builds BatchClient(endpoint=..., credential=...). azure-batch 15 sits on azure-core, whose retry policy reads retry_total (and retry_backoff_factor, retry_backoff_max, retry_status) from the client kwargs, so BatchClient(..., retry_total=n) is enough; I verified the pipeline's retry policy picks it up. The operator builds the hook in its cached hook property, so the natural path is a new optional hook argument that the operator fills from batch_max_retries.
    • One decision to make up front: the operator's documented default is 3, but azure-core's default retry_total is 10. Passing the value through unconditionally would change behaviour for every existing user (10 retries become 3). Defaulting both the operator parameter and the new hook argument to None (meaning "SDK default") keeps behaviour unchanged unless someone sets the parameter. I would lean to that, but a maintainer may prefer honouring the documented 3, so it is worth stating the choice in the PR description.
    • For tests, patching BatchClient in the hook module and asserting the constructor kwargs is enough; no live client needed.

    Airflow does not assign issues: just open the PR, mention this issue in it, and I am happy to review.

  3. shahar1
    added
    kind:taskA task that needs to be completed as part of a larger issue
    area:providers
    on Sep 13, 2026
  4. shahar1
    added the
    Task
    issue type on Sep 13, 2026

  5. namanjain24-sudo commented on Sep 21, 2026

    @namanjain24-sudo
    namanjain24-sudo
    on Sep 21, 2026
    Contributor

    Opened #73455 for the EksDeleteClusterOperator.execute_complete item: EksDeleteClusterTrigger yields "deleted", so the check now uses that status, with a regression test. It does not touch any of the other items in this list.

    Drafted-by: a Gen-AI coding assistant (no human review before posting)

  6. garvit-arora commented on Sep 29, 2026

    @garvit-arora
    garvit-arora
    on Sep 29, 2026

    Hi, I'd like to pick up the DataflowConfiguration.max_num_workers item from this umbrella issue. Plan: pass max_num_workers into Beam/Dataflow pipeline options where other DataflowConfiguration fields are applied, and add a focused provider unit test. I'll open a PR referencing this issue.

  7. bingqin2 commented on Sep 30, 2026

    @bingqin2
    bingqin2
    on Sep 30, 2026
    ContributorAuthor

    Hi @garvit-arora, go ahead, it's yours. A few pointers from when I wrote this up:

    • The other DataflowConfiguration fields become pipeline options in BeamDataflowMixin.__get_dataflow_pipeline_options in providers/apache/beam/src/airflow/providers/apache/beam/operators/beam.py. Keys there are camelCase (serviceAccount, impersonateServiceAccount); the Python and Go operators convert them to snake_case (_init_pipeline_options(format_pipeline_options=True)), the Java operator passes them as they are. So maxNumWorkers, set only when max_num_workers is not None, should reach all three SDKs.
    • _init_pipeline_options applies the operator's own pipeline_options after the Dataflow ones, so an explicit max_num_workers / maxNumWorkers pipeline option would win. Worth stating in the docstring and covering in a test.
    • Unit tests live in providers/apache/beam/tests/unit/apache/beam/operators/test_beam.py. The native Python system tests (providers/google/tests/system/google/cloud/dataflow/example_dataflow_native_python.py) already pass max_num_workers: 1 through dataflow_config, so today they silently ignore it; after the fix they exercise it.

  8. garvit-arora commented on Sep 30, 2026

    @garvit-arora
    garvit-arora
    on Sep 30, 2026

    Thanks for confirming the max_num_workers item is mine to work on — I'm finishing #73922 (latest push addresses the Java maxNumWorkers review feedback and adds doc + override test coverage).

    If maintainers would like another item from this umbrella issue picked up after that lands, I'm happy to take one of the still-open ones once direction is clear:

    • SparkKubernetesOperator.success_run_history_limit — wire or deprecate?
    • BigtableDeleteTableOperator.app_profile_id — wire or deprecate?
    • DbtCloudJobRunSensor.execute_complete handling for the latent "timeout" status

    Happy to follow whichever you or the maintainers would prefer reviewed next.

  9. namanjain24-sudo commented on Sep 30, 2026

    @namanjain24-sudo
    namanjain24-sudo
    on Sep 30, 2026
    Contributor

    The DbtCloudJobRunSensor.execute_complete timeout item looks already resolved — #69739 (merged 2026-09-24) rewrote the status check to if event["status"] != "success": raise ..., which covers "timeout" along with any other non-success status. Might be worth ticking off above.

    Drafted-by: Claude Code (Sonnet 5)

  10. bingqin2 commented on Oct 5, 2026

    @bingqin2
    bingqin2
    on Oct 5, 2026
    ContributorAuthor

    Thanks both. I ticked the dbt item, since #69739 covers it, and linked #73922 for the Dataflow item.

    For the two remaining items, my read is to deprecate both, because neither can work as documented: successfulRunHistoryLimit exists only on ScheduledSparkApplication, not on the SparkApplication the operator submits, and the Bigtable admin API takes no app profile when deleting a table. @potiuk @shahar1, does deprecating them sound right? If so, @garvit-arora is welcome to take both once #73922 lands.

  11. garvit-arora commented on Oct 8, 2026

    @garvit-arora
    garvit-arora
    on Oct 8, 2026

    Hi @shahar1 and maintainers, a gentle follow-up on #73922. I addressed the Java maxNumWorkers naming feedback and added the documented override-precedence test. The PR is ready for review when convenient. Thanks!

  12. nayansharma8123-gif commented on Oct 24, 2026

    @nayansharma8123-gif
    nayansharma8123-gif
    on Oct 24, 2026

    Hi @bingqin2,

    I'd like to contribute the BigtableDeleteTableOperator.app_profile_id item from this issue.

    I checked the current implementation: the parameter is accepted and stored by the operator, but it is not used when calling BigtableHook.delete_table(). I also saw your suggestion to deprecate this parameter because the Bigtable admin API does not accept an app profile for table deletion.

    I'm interested in implementing the deprecation with appropriate tests and documentation. I noticed @garvit-arora had also expressed interest in this item after #73922 lands. Would it be okay for me to work on this now, or would you prefer that I wait?

    Thanks!

  13. bingqin2 commented on Oct 25, 2026

    @bingqin2
    bingqin2
    on Oct 25, 2026
    ContributorAuthor

    Hi @nayansharma8123-gif, sure, go ahead with BigtableDeleteTableOperator.app_profile_id. @garvit-arora, that leaves the Spark item for you once #73922 lands, if you still want it.

    No maintainer has confirmed the deprecation yet, so please explain in the PR why the parameter can't be wired (the admin API's table delete takes no app profile), and let the reviewer decide. The usual pattern is to keep the argument, warn with AirflowProviderDeprecationWarning when it is passed, and note the deprecation in the docstring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:providerskind:taskA task that needs to be completed as part of a larger issue

    Type

    Task

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions