Skip to content

[Draft] Lazy load Slack SDK In Slack provider#66028

Closed
dwreeves wants to merge 1 commit into
apache:mainfrom
dwreeves:lazy-load-slack-sdk
Closed

[Draft] Lazy load Slack SDK In Slack provider#66028
dwreeves wants to merge 1 commit into
apache:mainfrom
dwreeves:lazy-load-slack-sdk

Conversation

@dwreeves
Copy link
Copy Markdown
Contributor

Lazy-load slack_sdk imports to reduce memory footprint of both importing the Slack hooks and also usage (since some of the memory footprint is specific to the async SDK, which isn't always used even during execution contexts).


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

No, the code written by hand, but I used generative AI to write the benchmarking script in the issue #66025.


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {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.

@dwreeves dwreeves force-pushed the lazy-load-slack-sdk branch from c2b77a8 to 51b259a Compare April 28, 2026 16:00
Copy link
Copy Markdown
Contributor

@SameerMesiah97 SameerMesiah97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As evidenced by the numerous CI failures, it looks like this PR has broken patching in multiple tests. I would ensure that patching is applied correctly for each runtime codepath under your new implementation. Until then, it would be best to keep this as a draft PR.

@dwreeves
Copy link
Copy Markdown
Contributor Author

@SameerMesiah97 really sorry about that, I did the work on my laptop and couldn't get the CI working locally, not even prek. I'll fix at home this evening on my personal laptop where Airflow dev environment is set up.

@dwreeves dwreeves changed the title Lazy load Slack SDK In Slack provider [Draft] Lazy load Slack SDK In Slack provider Apr 28, 2026
@SameerMesiah97
Copy link
Copy Markdown
Contributor

@SameerMesiah97 really sorry about that, I did the work on my laptop and couldn't get the CI working locally, not even prek. I'll fix at home this evening on my personal laptop where Airflow dev environment is set up.

No need to apologize. This was just a heads up because a maintainer would likely ask you to convert it to draft anyway. Take as much time as you need.

@potiuk potiuk marked this pull request as draft May 11, 2026 01:21
@potiuk
Copy link
Copy Markdown
Member

potiuk commented May 11, 2026

@dwreeves Converting to draft — this PR doesn't yet meet our Pull Request quality criteria.

  • Pre-commit / static checks — Failing: CI image checks / Static checks. See docs.
  • Provider tests — Failing: 4 provider distributions tests / Compat runs against Airflow 2.11.1, 3.0.6, 3.1.8, 3.2.1 (P3.10). See docs.

See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is not a rejection — just an invitation to bring the PR up to standard. No rush.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiuk
Copy link
Copy Markdown
Member

potiuk commented May 18, 2026

@dwreeves This draft PR has been inactive for 7 days since the last triage comment and no response from the author. Closing to keep the queue clean.

You are welcome to reopen this PR when you resume work, or to open a new one addressing the issues previously raised. There is no rush — take your time.

Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

@potiuk potiuk closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] [Slack Provider] Lazy import of Slack modules in slack/hooks/*

3 participants