[AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor #7407

dferguson992 · 2020-02-12T19:00:39Z

Dear Airflow Maintainers,

Please accept the following PR that

Add the KafkaProducerHook.
Add the KafkaConsumerHook.
Add the KafkaSensor which listens to messages with a specific topic.
Related Issue:
#1311

Issue link: AIRFLOW-6786

Make sure to mark the boxes below before creating PR: [x]

Description above provides context of the change
Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
Unit tests coverage for changes (not needed for documentation changes)
Commits follow "How to write a good git commit message"
Relevant documentation is updated including usage instructions.
I will engage committers as explained in Contribution Workflow Example.

For document-only changes commit message can start with [AIRFLOW-XXXX].

Reminder to contributors:

You must add an Apache License header to all new files
Please squash your commits when possible and follow the 7 rules of good Git commits
I am new to the community, I am not sure the files are at the right place or missing anything.

The sensor could be used as the first node of a dag where the second node can be a TriggerDagRunOperator. The messages are polled in a batch and the dag runs are dynamically generated.

Thanks!

Note, as per denied PR #1415, it is important to mention these integrations are not suitable for low-latency/high-throughput/streaming. For reference, #1415 (comment).

Co-authored-by: Dan Ferguson [email protected]
Co-authored-by: YuanfΞi Zhu

boring-cyborg · 2020-02-12T19:00:42Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: [email protected]
Slack: https://apache-airflow-slack.herokuapp.com/

ashb · 2020-02-12T20:12:27Z

@dferguson992 This PR pre-dates the projects use of Jira so you'll need to sign up for an account at issues.apache.org/jira/ and create one, please.

ashb · 2020-02-12T20:39:29Z

@dferguson992 Good work!. I'll take a look in detail tomorrow. (there will likely be a few rounds of review as this code was quite old)

dferguson992 · 2020-02-12T20:48:14Z

@ashb thanks! i'll be on vacation, back on Tuesday of next week. Happy to make any adjustments or additions to the code-base once I return. The code is quite old you're right, but it should be pretty forward compatible with vanilla Kafka.

I'm not sure if this project shies away from branding, but it may be worthwhile to consider adding support for Confluent Kafka specifically.

ashb · 2020-02-12T20:55:31Z

The first thing I notice is that we now need to rename this:

airflow/contrib/hooks/kafka_consumer_hook.py

->

airflow/providers/apache/kafka/hooks/kafka_consumer.py

etc. A bit more detail is in https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths, shout if that's not clear.

ashb

I also think the "Co-authored-by" in the messages needs email address (we can use github ones) to make it attribute correclty.

airflow/contrib/hooks/kafka_consumer_hook.py

airflow/contrib/hooks/kafka_producer_hook.py

airflow/contrib/sensors/kafka_sensor.py

dferguson992 · 2020-02-18T14:12:59Z

@ashb I've made the recommended changes except for the github email address. How does one acquire someone else's GH email?

ashb · 2020-02-21T16:31:26Z

@dferguson992 I've force pushed this branch to update the format/add the emails.

I couldn't see where you got "Hanan Shteingart" from as an author- which PR was that?

airflow/contrib/hooks/kafka_consumer_hook.py

dferguson992 · 2020-03-11T22:31:42Z

hey @ashb I'm not sure what else to do for this PR. Is there anything outstanding before it can be merged? I fixed as many of the travis CI issues as I could not sure what else to do for this

ashb · 2020-03-13T13:06:35Z

@dferguson992 I've force pushed this branch to update the format/add the emails.

I couldn't see where you got "Hanan Shteingart" from as an author- which PR was that?

@dferguson992 Can you answer this Q?

dferguson992 · 2020-03-13T13:48:50Z

@ashb I got that co-author by mistake. on the original PR, that user was the last to comment on it. but i mistakenly took that user as the PR author, not an interested third-party as I should have. I've removed that user from the PR notes and the corresponding issue.

dferguson992 · 2020-03-31T13:10:25Z

Hey @ashb hope all is well. I'm not sure what to do to get the Travis CI checks to pass, but the code is functional and includes your suggestions. Can we move forward with the PR?

dmmakita · 2020-04-12T00:36:04Z

Hi, @dferguson992, I'm having similar needs related to airflow-kafka. May I try to help you with this issue #7407?

dferguson992 · 2020-04-13T14:46:08Z

Hi, @dferguson992, I'm having similar needs related to airflow-kafka. May I try to help you with this issue #7407?

That would be awesome! But I'm not sure what else needs doing. If you know more on this, please do go for it.

dmmakita · 2020-04-14T01:53:21Z

Thank you for your response. I will try to do something.

gumartinm · 2020-04-28T08:37:00Z

Looking forward to seeing this change in Airflow.

kaxil · 2020-04-28T12:48:46Z

Can you please rebase to the latest Master?

dferguson992 · 2020-04-28T13:10:41Z

@ashb can you please review? I've made the necessary changes to this PR and there seems to be growing demand for it. At this time I see there needs to a review by you and some modifications to the travis ci build which is failing for an unknown reason.

kaxil · 2020-04-28T13:15:12Z

@ashb can you please review? I've made the necessary changes to this PR and there seems to be growing demand for it. At this time I see there needs to a review by you and some modifications to the travis ci build which is failing for an unknown reason.

Can you rebase the PR please to the latest master, we have moved to Github Actions for CI for most of our tests.

dferguson992 · 2020-04-28T13:17:39Z

@ashb can you please review? I've made the necessary changes to this PR and there seems to be growing demand for it. At this time I see there needs to a review by you and some modifications to the travis ci build which is failing for an unknown reason.

Can you rebase the PR please to the latest master, we have moved to Github Actions for CI for most of our tests.

I don't have that as an option. It says merging is blocked. Should I create a new PR?

kaxil · 2020-04-28T13:27:06Z

@ashb can you please review? I've made the necessary changes to this PR and there seems to be growing demand for it. At this time I see there needs to a review by you and some modifications to the travis ci build which is failing for an unknown reason.

Can you rebase the PR please to the latest master, we have moved to Github Actions for CI for most of our tests.

I don't have that as an option. It says merging is blocked. Should I create a new PR?

git fetch upstream master
git rebase upstream/master

and force push your branch

dferguson992 · 2020-04-28T13:37:42Z

@ashb can you please review? I've made the necessary changes to this PR and there seems to be growing demand for it. At this time I see there needs to a review by you and some modifications to the travis ci build which is failing for an unknown reason.

Can you rebase the PR please to the latest master, we have moved to Github Actions for CI for most of our tests.

I don't have that as an option. It says merging is blocked. Should I create a new PR?
git fetch upstream master
git rebase upstream/master
and force push your branch

Okay so I've done that and the pre-checks have all gone. The only issue now is there's an old request for changes from ashb from Feb12th in a stuck state, and I can't seem to remove ashb as a reviewer from this PR

dferguson992 · 2020-05-06T12:17:25Z

Okay, my latest error in this build process is this:

/opt/airflow/docs/_api/airflow/providers/apache/kafka/hooks/kafka_consumer_hook/index.rst:37: WARNING: Unexpected indentation.
/opt/airflow/docs/_api/airflow/providers/apache/kafka/sensors/kafka_sensor/index.rst:35: WARNING: Field list ends without a blank line; unexpected unindent.
looking for now-outdated files... none found
pickling environment... done
checking consistency... /opt/airflow/docs/_api/airflow/providers/apache/kafka/hooks/index.rst: WARNING: document isn't included in any toctree
/opt/airflow/docs/_api/airflow/providers/apache/kafka/sensors/index.rst: WARNING: document isn't included in any toctree
done

Does anyone know how to resolve an issue like this? @ashb? @kaxil?

serkef · 2020-05-06T15:37:20Z

Okay, my latest error in this build process is this:

/opt/airflow/docs/_api/airflow/providers/apache/kafka/hooks/kafka_consumer_hook/index.rst:37: WARNING: Unexpected indentation.
/opt/airflow/docs/_api/airflow/providers/apache/kafka/sensors/kafka_sensor/index.rst:35: WARNING: Field list ends without a blank line; unexpected unindent.
looking for now-outdated files... none found
pickling environment... done
checking consistency... /opt/airflow/docs/_api/airflow/providers/apache/kafka/hooks/index.rst: WARNING: document isn't included in any toctree
/opt/airflow/docs/_api/airflow/providers/apache/kafka/sensors/index.rst: WARNING: document isn't included in any toctree
done

Does anyone know how to resolve an issue like this? @ashb? @kaxil?

@dferguson992 for the last 2 warnings, you need to add the new path in sphinx. Check the known issues in contributing.md
for the first 2 warnings, I'll add comments in the code where I think this fails. You test locally by building the docs.

airflow/providers/apache/kafka/hooks/kafka_consumer_hook.py

Co-authored-by: Dan Ferguson [email protected] Co-authored-by: Hanan Shteingart Co-authored-by: YuanfΞi Zhu

turbaszek · 2020-08-25T07:18:39Z

Not sure if this CI error about cyclic imports is related... @potiuk

Co-authored-by: Dan Ferguson [email protected] Co-authored-by: Hanan Shteingart Co-authored-by: YuanfΞi Zhu

…subdags

knackjason · 2021-08-16T18:25:29Z

Did this ever get merged in? I'd be interested in having officially supported Kafka hooks.

potiuk · 2021-08-16T18:41:20Z

Does not look like

potiuk · 2021-08-16T18:42:30Z

But there is this PR in progress: #12388

knackjason · 2021-08-16T18:44:34Z

But there is this PR in progress: #12388

Ah, I didn't realize there was a newer PR. Thanks, @potiuk!

dferguson992 mentioned this pull request Feb 12, 2020

Add KafkaConsumerHook and KafkaSensor #1415

Closed

dferguson992 changed the title ~~Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor~~ [AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor Feb 12, 2020

ashb requested changes Feb 12, 2020

View reviewed changes

dferguson992 requested a review from ashb February 18, 2020 14:12

kaxil reviewed Feb 24, 2020

View reviewed changes

airflow/contrib/hooks/kafka_consumer_hook.py Outdated Show resolved Hide resolved

boring-cyborg bot added the provider:Apache label Mar 11, 2020

serkef reviewed May 6, 2020

View reviewed changes

airflow/providers/apache/kafka/hooks/kafka_consumer_hook.py Show resolved Hide resolved

dan added 8 commits August 24, 2020 22:29

Modified file perms

3d44718

Modified file perms

1085084

Updated requirements files again

a6d55a4

Fixed indentation issue

b755869

Removed trailing whitespace

8262de2

Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor

033cd6c

Co-authored-by: Dan Ferguson [email protected] Co-authored-by: Hanan Shteingart Co-authored-by: YuanfΞi Zhu

Attempted to rebase?

d4af358

??

5dbd736

dan added 16 commits August 25, 2020 14:18

Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor

f5ec571

Co-authored-by: Dan Ferguson [email protected] Co-authored-by: Hanan Shteingart Co-authored-by: YuanfΞi Zhu

Modifying files in accordance with peer review

cbfd33d

Removed unused imports

62f3c57

Fixed isort issue

b69e5ee

adding more weird local files

5924491

Modifying readme to trigger builds

7708186

Updated readme to trigger build

6edd921

Fixed yaml linting issues

bf12172

Modifying readme to trigger builds

f2b7f97

Updated readme to trigger build

3a24cc1

updating dependencies files

a03c992

Modified file perms

3e32050

Removed legacy hook and sensor code

f9f1fb8

??????

f26f880

squash! [AIRFLOW-5500] Fix the trigger_dag api in the case of nested …

e245bed

…subdags

Trying to get this to work

15835e9

dferguson992 closed this Aug 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor #7407

[AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor #7407

dferguson992 commented Feb 12, 2020 •

edited

Loading

boring-cyborg bot commented Feb 12, 2020

ashb commented Feb 12, 2020

ashb commented Feb 12, 2020

dferguson992 commented Feb 12, 2020

ashb commented Feb 12, 2020

ashb left a comment

dferguson992 commented Feb 18, 2020

ashb commented Feb 21, 2020

dferguson992 commented Mar 11, 2020

ashb commented Mar 13, 2020

dferguson992 commented Mar 13, 2020

dferguson992 commented Mar 31, 2020

dmmakita commented Apr 12, 2020

dferguson992 commented Apr 13, 2020

dmmakita commented Apr 14, 2020

gumartinm commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

dferguson992 commented May 6, 2020

serkef commented May 6, 2020

turbaszek commented Aug 25, 2020

knackjason commented Aug 16, 2021

potiuk commented Aug 16, 2021

potiuk commented Aug 16, 2021

knackjason commented Aug 16, 2021

[AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor #7407

[AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor #7407

Conversation

dferguson992 commented Feb 12, 2020 • edited Loading

boring-cyborg bot commented Feb 12, 2020

ashb commented Feb 12, 2020

ashb commented Feb 12, 2020

dferguson992 commented Feb 12, 2020

ashb commented Feb 12, 2020

ashb left a comment

Choose a reason for hiding this comment

dferguson992 commented Feb 18, 2020

ashb commented Feb 21, 2020

dferguson992 commented Mar 11, 2020

ashb commented Mar 13, 2020

dferguson992 commented Mar 13, 2020

dferguson992 commented Mar 31, 2020

dmmakita commented Apr 12, 2020

dferguson992 commented Apr 13, 2020

dmmakita commented Apr 14, 2020

gumartinm commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

kaxil commented Apr 28, 2020

dferguson992 commented Apr 28, 2020

dferguson992 commented May 6, 2020

serkef commented May 6, 2020

turbaszek commented Aug 25, 2020

knackjason commented Aug 16, 2021

potiuk commented Aug 16, 2021

potiuk commented Aug 16, 2021

knackjason commented Aug 16, 2021

dferguson992 commented Feb 12, 2020 •

edited

Loading