-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add Azure Service Bus Queue and Subscription triggers for async message processing #53356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Azure Service Bus Queue and Subscription triggers for async message processing #53356
Conversation
…c message processing and unit tests
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for the PR.
Would it be simpler if the BaseAzureServiceBusTrigger just raised the TriggerEvent with the service_bus_client directly?
I understand the idea behind using asyncio.Queue here, but IMO, we don't need to be constrained by the current methods available in MessageHook or use an asyncio.Queue just to accommodate the MessageHook interface.
|
Hi, Sorry for the Delayed Response.
it would be simpler the only reason we have the base class is so that we can handle both the queue's and subscriptions and since that their hooks are different.
what is the best way you would recommend? I built it around the hooks so that it is easier but it is a bit more coupled together |
IMHO, add a new method for just receive message without execution callback can make the trigger more simpler.
airflow/providers/redis/src/airflow/providers/redis/triggers/redis_await_message.py Lines 61 to 72 in 4b3b62d
|
|
hi, thanks for the recommendation. i looked into the How should I proceed? thanks :) |
|
Sorry for the late reply.
IMO, this is what I mention "no need to be restricted with existed implementation of
Yes, so this is why I suggest to add a new method ( maybe named as WDYT? |
|
hi, thanks for the clarification, i had misunderstood what you meant by the |
Oh, sorry I should have described it more clearly. |
sure will do, and sorry for not asking for clarification before :) |
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! LGTM overall.
It seems some part of the documentation should be covered in further Queue Provider PR.
Just FYI, the "Queue URI" should be replace by "scheme" in further Queue Provider PR.
You may refer to 5c03795 as an example of how to adapt to “scheme”.
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Show resolved
Hide resolved
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Outdated
Show resolved
Hide resolved
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Outdated
Show resolved
Hide resolved
…e/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
…e/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
…e/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
… Subscription Providers
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI failure still need to be resolved, thanks!
- Replace Mock object string representations with proper Mock(body=value) pattern - Fix str(message.body) calls returning mock object strings instead of expected message content - Apply standard Azure provider mocking conventions consistent with other tests - Remove unnecessary comments and custom MockMessage classes
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for the update! LGTM overall.
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/asb.py
Outdated
Show resolved
Hide resolved
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/asb.py
Show resolved
Hide resolved
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Outdated
Show resolved
Hide resolved
vincbeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be done as a follow-up PR, but it would be also nice to add these 2 services as part of the unified MessageQueueTrigger . Please read documentation on how to do it.
The goal is to have one MessageQueueTrigger that is compatible with different queue providers, today we have SQS and Kafka. Would be awesome to add Azure as well
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Outdated
Show resolved
Hide resolved
|
@dabla or @bolkedebruin Do you have any knowledge about Azure Service Bus to review? |
Hallo @jscheffl , I don’t have any knowledge about this one but can also look into it, but I do agree with comments @vincbeck though! It should be implemented in such a way that it can be used by the generic MessgeQueueTrigger, otherwise we will end up having specific implementations per provider like with the SQL operators in the past, it should be provider agnostic. I actually did the same for MongoDB, still have to open a PR for that one. |
hi, can we take it up as a follow up PR? i have addressed the other comment that was added, anything else that needs to be worked on within this PR? :) lmk thanks! and sorry for the delay |
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update!
LGTM if the CI pass.
hi, can we take it up as a follow up PR?
Sure! We definitely can take this one just for adding Trigger of Azure Service Bus Queue, then the MessageQueueTrigger can be done in follow-up one.
providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py
Outdated
Show resolved
Hide resolved
…e/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have no big experience with Azure Service Bus but it looks sound and there are no red flags.
…ge processing (apache#53356) * Add Azure Service Bus integration with triggers and documentation * Implement Azure Service Bus Queue and Subscription triggers with async message processing and unit tests * feat(Azure): Add methods to read messages from Service Bus queue and subscription * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Remove Queue URI format documentation for Azure Service Bus Queue and Subscription Providers * Enhance Azure Service Bus integration by adding new message types and updating trigger initialization tests * Fix Azure Service Bus message trigger test failures - Simplified mocking to avoid connection and import issues - Fixed mock assertions for MessageHook initialization - Addressed compatibility issues with older Airflow versions - Used context managers for proper mock lifecycle management * Fix Azure Service Bus trigger test mocking issues - Replace Mock object string representations with proper Mock(body=value) pattern - Fix str(message.body) calls returning mock object strings instead of expected message content - Apply standard Azure provider mocking conventions consistent with other tests - Remove unnecessary comments and custom MockMessage classes * Update the Trigger Event Message to Include the Actual Content * resolve review comments * chore: change BaseTrigger to BaseEventTrigger * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * chore: fix ci/cd --------- Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
…ge processing (apache#53356) * Add Azure Service Bus integration with triggers and documentation * Implement Azure Service Bus Queue and Subscription triggers with async message processing and unit tests * feat(Azure): Add methods to read messages from Service Bus queue and subscription * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * Remove Queue URI format documentation for Azure Service Bus Queue and Subscription Providers * Enhance Azure Service Bus integration by adding new message types and updating trigger initialization tests * Fix Azure Service Bus message trigger test failures - Simplified mocking to avoid connection and import issues - Fixed mock assertions for MessageHook initialization - Addressed compatibility issues with older Airflow versions - Used context managers for proper mock lifecycle management * Fix Azure Service Bus trigger test mocking issues - Replace Mock object string representations with proper Mock(body=value) pattern - Fix str(message.body) calls returning mock object strings instead of expected message content - Apply standard Azure provider mocking conventions consistent with other tests - Remove unnecessary comments and custom MockMessage classes * Update the Trigger Event Message to Include the Actual Content * resolve review comments * chore: change BaseTrigger to BaseEventTrigger * Update providers/microsoft/azure/src/airflow/providers/microsoft/azure/triggers/message_bus.py Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> * chore: fix ci/cd --------- Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
Add Azure Service Bus Queue and Subscription triggers
This PR adds two new triggers for monitoring Azure Service Bus:
AzureServiceBusQueueTrigger- monitors Service Bus queues for messagesAzureServiceBusSubscriptionTrigger- monitors topic subscriptions for messagesThese triggers enable event-driven DAG execution when messages arrive in Azure Service Bus, supporting async message processing workflows.
Changes
Testing
related: AIP-82 (#52712)