Skip to content

[FLINK-40149][connector-http] Rename asyncPolling to async-polling#53

Open
bharathgunapati wants to merge 2 commits into
apache:mainfrom
bharathgunapati:FLINK-40149-async-polling-rename
Open

[FLINK-40149][connector-http] Rename asyncPolling to async-polling#53
bharathgunapati wants to merge 2 commits into
apache:mainfrom
bharathgunapati:FLINK-40149-async-polling-rename

Conversation

@bharathgunapati

Copy link
Copy Markdown
Contributor

What is the purpose of the change

The HTTP lookup option for sync vs async polling is the only camelCase table
option in the connector (asyncPolling). All other options use kebab-case or
dotted keys (e.g. lookup-method, http.source.lookup.request.timeout).

This renames the canonical key to async-polling to align with Flink connector
naming conventions. The legacy asyncPolling key remains supported via
withDeprecatedKeys so existing DDL does not need to change.

No runtime behavior change: default remains false, and the option is already
read via ReadableConfig.get(ASYNC_POLLING).

Brief change log

  • HttpLookupConnectorOptions.ASYNC_POLLING: canonical key async-polling,
    deprecated alias asyncPolling, description updated.
  • Docs (en + zh): option table and examples updated to async-polling.
  • Tests: parameterized factory and runtime tests cover both canonical and
    deprecated keys; ITCases updated to canonical key (mechanical).

Compatibility note

  • Canonical key: async-polling
  • Legacy key: asyncPolling still works via withDeprecatedKeys
  • Default: false (unchanged)
  • Migration: optional — existing DDL using asyncPolling continues to work;
    new configurations should use async-polling

Release note is on FLINK-40149.

Verifying this change

CI green on fork (Flink 2.2.0, JDK 11/17/21):
https://github.com/bharathgunapati/flink-connector-http/actions/runs/29353591657

Automated tests:

  • HttpLookupTableSourceFactoryTest#shouldAcceptAsyncPollingAtFactory — factory
    accepts both async-polling and asyncPolling
  • HttpLookupTableSourceTest#shouldCreateAsyncTableSourceWithAsyncPollingKey
    runtime async path for both keys

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: no

Documentation

  • Does this pull request introduce a new feature? no (consistency improvement)
  • If yes, how is the feature documented? docs updated (en + zh)

Rename the canonical lookup option key to async-polling to align with Flink
connector kebab-case conventions. The legacy asyncPolling key remains supported
via withDeprecatedKeys for backward-compatible DDL.

Updates docs, examples, and tests to use the new canonical key. Adds factory-
and runtime-level parameterized tests for both the canonical key and the
deprecated alias. Documents the alias in the ConfigOption description and docs.
@bharathgunapati

Copy link
Copy Markdown
Contributor Author

Hi @davidradl — would you be able to review this when you have a moment?

Small consistency improvement (FLINK-40149): renames the lookup option asyncPollingasync-polling to match Flink kebab-case conventions. The legacy key still works via withDeprecatedKeys("asyncPolling"), so no DDL migration is required.

Scope: option key + docs + tests only — no runtime logic change (default remains false, still read via ReadableConfig).

Tests: parameterized factory and runtime tests cover both async-polling and the deprecated asyncPolling alias.

CI: green on my fork (Flink 2.2.0, JDK 11/17/21) — https://github.com/bharathgunapati/flink-connector-http/actions/runs/29353591657

Thanks!

| format | required | Flink's format name that should be used to decode REST response, Use `json` for a typical REST endpoint. |
| url | required | The base URL that should be used for GET requests. For example _http://localhost:8080/client_ |
| asyncPolling | optional | true/false - determines whether Async Polling should be used. Mechanism is based on Flink's Async I/O. |
| async-polling | optional | true/false - determines whether async polling should be used. Mechanism is based on Flink's Async I/O. Deprecated alias: `asyncPolling`. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now we have release we cannot easily change a config name . We would need to deprecate but keep the old property and indicate people should use the new property name in preference. So we need to keep the docs for asyncPolling and still accept that property. We probably need some validation to reject configs with both keys set

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the review feedback — pushed 0b4cb59:

  • Docs list both keys: async-polling (preferred) and asyncPolling (deprecated, still accepted)
  • Factory rejects DDL that sets both keys (ValidationException)
  • asyncPolling still works at runtime via withDeprecatedKeys — no behavior change for existing DDL

… rename

Document async-polling as preferred and asyncPolling as deprecated in both
connector docs, reject DDL that sets both keys, and keep the ConfigOption
description focused on behavior only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants