apache / apache/airflow

BaseMessageQueueProvider declares abstractmethods without ABC, and three in-tree providers do not implement them

Closed
#72,916 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
46.9k
Forks
17.8k
Avg merge
2d 9h
Merged PRs (30d)
472

Description

### Body

While adding the missing test module for `common.messaging` (#72915, part of #35442) I found the provider base-class contract is out of sync with its implementations:

**1. The abstract contract is not enforced.** `BaseMessageQueueProvider` decorates `queue_matches` / `trigger_class` / `trigger_kwargs` with `@abstractmethod`, but the class does not inherit `abc.ABC`, so nothing blocks instantiating an incomplete subclass. Calling an unimplemented "abstract" method silently returns `None`, which in `MessageQueueTrigger.trigger` dispatch makes the provider silently never match by queue instead of failing loudly.

**2. Three in-tree providers rely on that gap.** `PubSubMessageQueueEventTriggerContainer` (google), `RedisPubSubMessageQueueProvider` (redis), and `AzureServiceBusMessageQueueProvider` (microsoft.azure) implement only `trigger_class` + the `scheme` attribute — they support the scheme-based dispatch path only, and would fail to instantiate if `abc.ABC` were simply added. The older providers (amazon sqs, apache.kafka, ibm.mq) implement all three.

So the fix is a design decision rather than a mechanical change. Options as I see them:

- **(a)** Inherit `abc.ABC`, and demote `queue_matches` / `trigger_kwargs` from abstract to concrete defaults (`return False` / `return {}`) documented as "override for queue-URI dispatch" — keeps `trigger_class` as the only hard requirement, matches what the three scheme-only providers already assume, and no provider changes needed.
- **(b)** Inherit `abc.ABC` keeping all three abstract, and add the two missing methods to the three scheme-only providers.
- **(c)** Keep the class as-is and only fix the docstrings to say which methods are required for which dispatch path (no enforcement).

Happy to implement whichever direction maintainers prefer.

### Committer

- [ ] I acknowledge that I am a maintainer/committer of the Apache Airflow project.

Contributor guide

Open the contributing guide

Research direction

Start in the common.messaging test work referenced by #72915 and inspect BaseMessageQueueProvider alongside MessageQueueTrigger.trigger. Compare the contract with PubSubMessageQueueEventTriggerContainer, RedisPubSubMessageQueueProvider, and AzureServiceBusMessageQueueProvider, then confirm the selected enforcement behavior with tests covering scheme- and queue-based dispatch and incomplete subclasses.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, data-engineering
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.