Add hook for amazonmsk#69000
Conversation
| # in the `devel-dependencies` section to be the same minimum version. | ||
| "boto3>=1.41.0", | ||
| "botocore>=1.41.0", | ||
| "aws-msk-iam-sasl-signer-python>=1.0.2", |
There was a problem hiding this comment.
The usage of this is optional in the code as you've written it, but it's included as a mandatory dependency here. Let's move this below to be an optional dependency
There was a problem hiding this comment.
Ok, I will modify it later
o-nikolas
left a comment
There was a problem hiding this comment.
We'd probably have to bump the botocore minimum since these are new APIs?
|
Quickest fix: git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-leaseAutomated nudge — ignore if you're not ready to rebase. This comment is updated in place on future |
|
Agreed with Niko. Looks like the hooks are generally replacing one-liners with functions, which we generally avoid. |
There was a problem hiding this comment.
Thanks, LGTM overall.
Most of the methods look like single line wrappers around the boto API. It seems like we don't have much of a usecase for this?
I agreed with this point as well. We could make the MSK Hook itself just the interface to retrieve the authenticated MSK client with given conn_id.
Same as the AwaitMessageTrigger (the Kafka concusmer in Airlfow trigger) implementation, we will leverage the SDK client that retrieved via connection directly without additional wrapper.
vincbeck
left a comment
There was a problem hiding this comment.
I agree, I dont think these methods are needed. They do not add much value, in the task/operator you can directly invoke boto3 the same way you do it in the hook
|
Thanks for reviewing. I agree that these methods are not necessary. I will follow @jason810496 comment and modify the PR and make it focus on retrieving authenticated MSK client |
deba9d9 to
016dcca
Compare
016dcca to
3d68ddd
Compare
What is this PR for?
Integrate Amazon MSK for user to have variety way to implement Event-Driven Scheduling feature which has been introduced in Airflow 3.0
related: #52712
What is changed in this PR?
This PR add a hook for Amazon MSK. We only make this hook as an interface for creating an authenticated MSK boto3 client so far.
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.