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
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.
BaseMessageQueueProviderdecoratesqueue_matches/trigger_class/trigger_kwargswith@abstractmethod, but the class does not inheritabc.ABC, so nothing blocks instantiating an incomplete subclass. Calling an unimplemented "abstract" method silently returnsNone, which inMessageQueueTrigger.triggerdispatch 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), andAzureServiceBusMessageQueueProvider(microsoft.azure) implement onlytrigger_class+ theschemeattribute — they support the scheme-based dispatch path only, and would fail to instantiate ifabc.ABCwere 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:
abc.ABC, and demotequeue_matches/trigger_kwargsfrom abstract to concrete defaults (return False/return {}) documented as "override for queue-URI dispatch" — keepstrigger_classas the only hard requirement, matches what the three scheme-only providers already assume, and no provider changes needed.abc.ABCkeeping all three abstract, and add the two missing methods to the three scheme-only providers.Happy to implement whichever direction maintainers prefer.
Committer