-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Refactor Common Queue Interface #54651
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
Refactor Common Queue Interface #54651
Conversation
|
I will take a look in a day or two. Some priority things are creeping in. |
providers/amazon/src/airflow/providers/amazon/aws/queues/sqs.py
Outdated
Show resolved
Hide resolved
providers/apache/kafka/src/airflow/providers/apache/kafka/queues/kafka.py
Outdated
Show resolved
Hide resolved
providers/amazon/src/airflow/providers/amazon/aws/queues/sqs.py
Outdated
Show resolved
Hide resolved
providers/apache/kafka/src/airflow/providers/apache/kafka/queues/kafka.py
Outdated
Show resolved
Hide resolved
providers/amazon/src/airflow/providers/amazon/aws/queues/sqs.py
Outdated
Show resolved
Hide resolved
- remove "New scheme-based approach" comment - remove redundent "the"
36dc699 to
6fc63ac
Compare
Agree. I will vote for "Sudden death" approach in this case and have same though as this If we decide to go with "Sudden death", does it mean we need to remove the compatible interface for common queue?
By the way, only SQS and Kafka will be impacted, since Queue Provider of Redis haven't been released before. It was excluded in last provider release. |
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.
Nice!
|
To avoid having new queue provider using the deprecated mechanism, I vote for the "sudden death" solution too |
|
Thanks @jason810496 ! |
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.
Yeah. I propose sudden death too :)
|
cc: @eladkal . This is yet another example where we have to (when you prepare providers) do the following: a) update common.messaging to 2.0.0 There are two options for it:
I am not sure if there is another way of storing it - or just remembering that you have to do it is enough. Or maybe we can figure out a way how to replace "remembering" with some information stored in the repo. Currently we have no such information stored, what we should do is literally stored in this discussion. Maybe we might come up with some good ideas how to handle those cases better @gopidesupavan @amoghrajesh as well? |
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
|
@potiuk @eladkal I might have a simple proposal here without too much complexity. What if we add a simple coordination block at the top of provider changelogs to store actionable information that release tooling can process during the release? In short this is what I propose, we define a block at the top of a provider's changelog (since this will be mostly be per provider granularity) Format# providers/common/messaging/docs/changelog.rst
.. COORDINATION-PENDING::
**Breaking Version:** 2.0.0
**Update Required:**
* apache-airflow-providers-amazon: ">=2.0.0"
* apache-airflow-providers-google: ">=2.0.0"
* apache-airflow-providers-azure: ">=2.0.0"
``apache-airflow-providers-common-messaging``
Changelog
---------
2.0.0
.....Workflow
Potential ImplementationWe will have to define some sort of parsing block to automatically updating provider dependencies during release preparation. Like define a python class like this -- Block: Parsing: Example: |
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
Yep. I thought about something similar. I think it is important to have it as simple as possible and natural for the developer to do. I think maybe a better idea though could be comments in Amazon pyproject.toml: [project.optional-dependencies]
"common.messaging" = [
"apache-airflow-providers-common-messaging>=1.0.3" # BUMPRedis pyproject.toml: [project.optional-dependencies]
"common.messaging" = [
"apache-airflow-providers-common-messaging>=1.0.3" # BUMP
]Maybe not exactly this comment, but smth like that should be more than enough to actually do the bump. It would be "post-processing" after prepare providers documentation is done. |
|
Also - I think this is a bit more "accurate" - becasue we do not know exactly what will be the bump. For example if we decided that common.messaging is not going to have a major release (i.e. non-sudden-death scenario) the # BUMP would mean |
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
* Replace queue URI with scheme for MessageQueueTrigger * Refactor KafkaMessageQueueProvider * Refactor SqsMessageQueueProvider * Refactor RedisPubSubMessageQueueProvider * Fix tests - Add test_utils/common_msg_queue - Fix unit tests for sqs, kafka, common messaging - Fix integration test for redis - Fix system test for redis * Fix fixture import for unit tests * fixup! Fix test_sqs * Remove docs for deprecated URI * Fix KafkaMessageQueueTrigger with scheme based usage * Update example dags * Fix doc nits - remove "New scheme-based approach" comment - remove redundent "the" * Fix Kafka TestMessageQueueTrigger
related: #52712
related discussion: #53556 (comment)
Why
Here are some advantages of using "scheme+passing keyword parameters directly from MessageQueueTrigger" instead of the "Queue URI" interface:
For more details, please refer to the discussion at: #53556 (comment)
What
queueURI parameter should be replaced by theschemeparameter for matching the provider's queue.AwaitMessageTrigger,SqsSensorTrigger) should be passed directly fromMessageQueueTrigger.queueURI.queueparameter in both logging and documentation.