') + ')', '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); } })(); })(); Cancel Dataproc submit triggers on user kill via on_kill() by raushanprabhakar1 · Pull Request #65743 · apache/airflow · GitHub
Skip to content

Cancel Dataproc submit triggers on user kill via on_kill() - #65743

Closed
raushanprabhakar1 wants to merge 1 commit into
apache:mainfrom
raushanprabhakar1:dataproc-on-kill
Closed

Cancel Dataproc submit triggers on user kill via on_kill()#65743
raushanprabhakar1 wants to merge 1 commit into
apache:mainfrom
raushanprabhakar1:dataproc-on-kill

Conversation

@raushanprabhakar1

@raushanprabhakar1raushanprabhakar1 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

DataprocSubmitTrigger and DataprocSubmitJobDirectTrigger now cancel in-flight Dataproc jobs via BaseTrigger.on_kill() when the user kills the deferred task, instead of catching asyncio.CancelledError in run() and using safe_to_cancel() to guess whether the triggerer was restarting. Polling in run() is unchanged aside from no longer handling cancellation there; cancel_on_kill and job_id still gate whether cancel_job runs.

Unit tests assert that CancelledError during polling does not call cancel_job, and that on_kill() performs the cancel (or skips when cancel_on_kill is false or job_id is missing).

related: #65733

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
  • 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.

@boring-cyborgboring-cyborgBot added area:providers provider:google Google (including GCP) related issues labels Apr 23, 2026
SameerMesiah97
SameerMesiah97 reviewed Apr 24, 2026
View reviewed changes

@SameerMesiah97SameerMesiah97 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.

I have left a few comments. I think you are removing too much code for no reason.

Comment threadproviders/google/src/airflow/providers/google/cloud/triggers/dataproc.py
},
)

if not AIRFLOW_V_3_0_PLUS:

@SameerMesiah97SameerMesiah97Apr 24, 2026

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.

I don't understand why you removed this. This will break cancellation semantics for this trigger for the Airflow 3.x versions before 3.30 (which appears to be the version where on_kill() is being introduced in BaseTrigger).

Comment threadproviders/google/src/airflow/providers/google/cloud/triggers/dataproc.py
# Database query is needed to get the latest state of the task instance.
task_instance = self.get_task_instance() # type: ignore[call-arg]
task_state = task_instance.state
return task_state != TaskInstanceState.DEFERRED

@SameerMesiah97SameerMesiah97Apr 24, 2026

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.

You’re removing quite a bit of existing code here, which introduces a backwards compatibility issue for pre-3.30 Airflow. In those versions on_kill() isn’t invoked, so cancellation would no longer go through. I’d have another look at PR #65672; there on_kill() is added without removing the existing CancelledError path.

I think it would be better to just add on_kill() (with tests) and leave the current logic in run() as-is. That way older versions continue to use the existing code path, and newer versions can make use of on_kill() when it’s available.

@potiuk

potiuk commented Apr 27, 2026

Copy link
Copy Markdown
Member

@raushanprabhakar1 This PR has a few issues that need to be addressed before it can be reviewed — please see our Pull Request quality criteria.

Issues found:

  • ❌ Pre-commit / static checks: Failing: CI image checks / Static checks. Run prek run --from-ref main --stage pre-commit locally to reproduce and fix the offending hooks.

What to do next:

  • Fix each issue listed above.
  • Make sure static checks pass locally (prek run --from-ref main --stage pre-commit).
  • Mark the PR as "Ready for review" when you're done.

There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

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.

@SameerMesiah97

SameerMesiah97 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Looks like this has already been implemented via PR #65742 which has been merged.

@shahar1

shahar1 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Looks like this has already been implemented via PR #65742 which has been merged.

Closing due to the above.
Thanks for your contribution - I hope that your next PR will be merged!

@shahar1shahar1 closed this Apr 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahar1shahar1Awaiting requested review from shahar1shahar1 is a code owner
1 more reviewer
@SameerMesiah97SameerMesiah97SameerMesiah97 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providersprovider:googleGoogle (including GCP) related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@raushanprabhakar1@potiuk@SameerMesiah97@shahar1