') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Convert check-sdk-imports to capture occurences than files by amoghrajesh · Pull Request #71212 · apache/airflow · GitHub
Skip to content

Convert check-sdk-imports to capture occurences than files - #71212

Merged
amoghrajesh merged 3 commits into
apache:mainfrom
amoghrajesh:strengthen-prek-sdk-imports
Aug 11, 2026
Merged

Convert check-sdk-imports to capture occurences than files#71212
amoghrajesh merged 3 commits into
apache:mainfrom
amoghrajesh:strengthen-prek-sdk-imports

Conversation

@amoghrajesh

@amoghrajeshamoghrajesh commented Aug 6, 2026

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

The hook was meant to stop airflow-core from gaining new runtime dependencies on the task SDK, but it worked by excluding whole files that already had an import when the check was added. That made it blind to new imports added later to any of those files - which is exactly what happened in #70370, where a new airflow.sdk import landed
in models/taskinstance.py unnoticed because the file was already on the exclude list for an older, unrelated import.

Track an allowed import count per file instead (same pattern as check_new_airflow_exception_usage.py), so existing file can't quietly gain more without either a # noqa: SDK001 or a deliberate allowlist regeneration.

  • 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.

@amoghrajesh
amoghrajesh requested review from Lee-W and kaxilAugust 6, 2026 05:26
@amoghrajeshamoghrajesh mentioned this pull request Aug 6, 2026
Merged
jason810496
jason810496 reviewed Aug 6, 2026
View reviewed changes

@jason810496jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Yeah, it makes sense to follow the know_....txt convention.

Comment threadgenerated/known_sdk_imports_in_core.txt Outdated
ferruzzi
ferruzzi reviewed Aug 6, 2026
View reviewed changes

@ferruzziferruzzi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two thoughts, non-blocking. Thanks for taking this on.

Comment threadscripts/ci/prek/check_sdk_imports_in_core.py Outdated
Comment threadscripts/ci/prek/check_sdk_imports_in_core.py
kaxil
kaxil approved these changes Aug 7, 2026
View reviewed changes
Comment threadscripts/ci/prek/check_sdk_imports_in_core.py
Comment threadgenerated/known_sdk_imports_in_core.txt
@amoghrajesh
amoghrajesh merged commit d8cfc95 into apache:mainAug 11, 2026
69 checks passed
@github-actions

github-actionsBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-3-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
✅v3-3-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request Aug 13, 2026
@amoghrajesh@vatsrahul1001
[v3-3-test] Convert check-sdk-imports to capture occurences than files (
0772378
#71212)
(cherry picked from commit d8cfc95)
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
dabla pushed a commit to dabla/airflow that referenced this pull request Aug 14, 2026
@amoghrajesh@dabla
Convert check-sdk-imports to capture occurences than files (apache#71212
50dc3ff
)
potiuk pushed a commit that referenced this pull request Aug 17, 2026
@amoghrajesh@potiuk
[v3-3-test] Convert check-sdk-imports to capture occurences than files (
213c22a
#71212)
(cherry picked from commit d8cfc95)
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
potiuk pushed a commit that referenced this pull request Aug 17, 2026
@github-actions@jason810496@amoghrajesh
[v3-3-test] Convert check-sdk-imports to capture occurences than files (
a75b7cd
#71212) (#71398)
* Consolidate AllowlistManager for prek hooks (#69327)
* Extract common AllowListManager base class for prek hooks
The allowlist load/save/generate/cleanup logic was duplicated across
check_new_airflow_exception_usage.py and check_provide_session_kwargs.py.
This consolidates the shared pattern into an abstract AllowListManager
in common_prek_utils.py, with subclasses providing only iter_files()
and count_occurrences().
* Move check loop and parse into AllowlistManager base class
The initial extraction left the scan/tighten/report loop duplicated
in both hook scripts and exposed `parse` as a staticmethod that
silently ignored the instance's `repo_root`.
- Convert `parse` from `@staticmethod` to instance method so it
always uses `self.repo_root` (fixes the latent footgun where
`_parse_tracked_allowlist` called parse unbound).
- Hoist the check loop into `AllowlistManager.check()` with
`violation_panel_text()` (abstract) and `format_violation_details()`
(overridable) hooks — both `_check_*` wrappers are now one-liners.
- Fix naming: `AllowListManager` → `AllowlistManager` to match the
subclass convention used throughout.
- Add test coverage for `check_new_airflow_exception_usage.py`.
(cherry picked from commit 7e47a52)
* [v3-3-test] Convert check-sdk-imports to capture occurences than files (#71212)
(cherry picked from commit d8cfc95)
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
---------
Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaxilkaxilkaxil approved these changes
@jason810496jason810496jason810496 approved these changes
@potiukpotiukAwaiting requested review from potiukpotiuk is a code owner
@ashbashbAwaiting requested review from ashbashb is a code owner
@gopidesupavangopidesupavanAwaiting requested review from gopidesupavangopidesupavan is a code owner
@jscheffljschefflAwaiting requested review from jscheffljscheffl is a code owner
@bugraoz93bugraoz93Awaiting requested review from bugraoz93bugraoz93 is a code owner
@Lee-WLee-WAwaiting requested review from Lee-W
@ferruzziferruzziAwaiting requested review from ferruzzi

Assignees

No one assigned

Labels

area:dev-toolsbackport-to-v3-3-testBackport to v3-3-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amoghrajesh@ferruzzi@kaxil@jason810496