You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org> on 2026/09/18 10:56:10 UTC

[PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Antonio-RiveroMartnez opened a new pull request, #44426:
URL: https://github.com/apache/superset/pull/44426

### SUMMARY

Separate Global Task Framework installation from request-time feature enablement.

- Add `GLOBAL_TASK_FRAMEWORK_ENABLED = False` as explicit, process-wide deployment configuration. Task API/view, TaskManager and task MCP installation use this setting, not a runtime feature-flag decision. No compatibility inference from static or dynamic flags.
- Require deployment configuration **and** effective `GLOBAL_TASK_FRAMEWORK` for Task API access, direct page/menu access, synchronous/asynchronous wrapper admission, the shared submission command, and MCP invocation. Existing authentication, Task permissions and subscription filters are unchanged.
- Require installed infrastructure and effective GTF/GAQ flags before selecting asynchronous chart execution; otherwise retain synchronous execution. Leave FeatureFlagManager and its GAQ-implies-GTF derivation unchanged.
- Expose the deployment boolean in public bootstrap configuration; gate the task list and async-query UI/request predicates explicitly.
- Document the breaking configuration change and lifecycle contract: stop new submissions and drain work before disabling effective GTF. Polling/cancellation are gated too, while workers can finish admitted work.

Admission audit: both wrapper paths and `TaskManager.submit_task` converge on `SubmitTaskCommand.run_with_info`. `TaskDAO.create_task` is the persistence primitive called by that command, not an independent scheduling entry point. Worker execution uses the registered executor directly and does not re-enter admission. Frontend response/completion handling remains intact for requests already admitted.

### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not included: this changes availability predicates, not the enabled UI layout. The Task List disabled-state rendering is covered by a component test; no live browser validation was performed.

### TESTING INSTRUCTIONS

1. Start with each combination of `GLOBAL_TASK_FRAMEWORK_ENABLED` and effective `GLOBAL_TASK_FRAMEWORK`. Config-off must not install task routes/tools. Config-on must install them regardless of the runtime flag.
2. With config on, switch a dynamic runtime flag off/on/off without restarting. Verify API access (including `_info`, list/detail, related/distinct, polling and cancellation), direct `/tasks/list/`, menu visibility and MCP invocation follow the runtime decision without changing route/permission registration. Existing RBAC/subscriber restrictions must still apply when enabled.
3. Submit chart requests with `async_mode=true`. Config-off, or a customized resolver returning GAQ true/GTF false, must use synchronous execution rather than return 202. With all prerequisites present, check cached and cold-cache queries, authenticated/guest identities, Task-read grants and task failures.
4. Admit a task, disable the effective runtime flag, and verify its worker can reach a terminal state while user-facing task access is denied. For operational rollout, drain first rather than using this as the normal disable procedure.
5. Run migrations and permission synchronization with matching deployment configuration, then restart web/worker/MCP processes. Verify both clean and existing metadata databases before deployment.

Automated validation:

- `pytest -q tests/unit_tests/tasks tests/unit_tests/charts/test_chart_data_api.py tests/unit_tests/mcp_service/test_mcp_tool_registration.py tests/unit_tests/mcp_service/task/tool/test_task_tools.py` — **605 passed**.
- `pytest -q tests/unit_tests/tasks/test_runtime_config.py` after the final synchronous-fallback cases — **33 passed** (overlaps the suite above).
- From `superset-frontend`: `npx jest --runInBand --silent src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx src/utils/asyncMode.test.ts src/pages/TaskList/TaskList.test.tsx` — **31 passed**.
- `pre-commit run` on staged changes — all applicable hooks passed **except frontend type-checking**. That check reports unbuilt workspace declaration outputs (`TS6305`, including core/chart-controls) and related type errors; it is not claimed passing. Frontend formatting/lint/custom-rules/stylelint and Python mypy/ruff passed.
- Direct pylint on changed backend source files — passed (10.00/10). `git diff --check` — passed.

Validation limitations: no running local Superset service, live Celery/Redis workflow, browser smoke test, or clean/existing metadata-database permission provisioning test. Frontend tests ran on Node 22 rather than the repository's Node 24 toolchain. Python test dependencies were installed without optional native MySQL/LDAP/SASL packages because their system build prerequisites were unavailable. Full integration and frontend type-check validation remain required.

### ADDITIONAL INFORMATION

- [ ] Has associated issue:
- [x] Required feature flags: existing `GLOBAL_TASK_FRAMEWORK`; `GLOBAL_ASYNC_QUERIES` for async charts. Both also require `GLOBAL_TASK_FRAMEWORK_ENABLED=True`.
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API

Breaking deployment configuration change; no schema or permission-name migration. This PR remains draft pending full integration validation.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #44426:
URL: https://github.com/apache/superset/pull/44426#discussion_r4062692690


##########
docs/admin_docs/configuration/async-queries-celery.mdx:
##########
@@ -113,3 +113,69 @@ celery --app=superset.tasks.celery_app:app flower
 
 - [Blog: How to Set Up Global Async Queries (GAQ) in Apache Superset](https://medium.com/@ngigilevis/how-to-set-up-global-async-queries-gaq-in-apache-superset-a-complete-guide-9d2f4a047559)
   :::
+
+## Task infrastructure, Tasks UI and asynchronous chart queries
+
+These three controls have different responsibilities:
+
+1. **`GLOBAL_TASK_FRAMEWORK_ENABLED` deployment config** (default `False`)
+   installs task infrastructure and enables task services: admission, Task APIs
+   (including polling/cancellation), TaskManager and task MCP tools.
+2. **`GLOBAL_TASK_FRAMEWORK` feature flag** controls only Tasks UI visibility:
+   the menu, direct Tasks page and frontend Task List. It also requires the config.
+   It is not a task execution/API kill switch or a security authorization control.
+3. **`GLOBAL_ASYNC_QUERIES` feature flag** enables async chart eligibility when
+   the config is on. It does not require the Tasks UI flag. Turning GAQ off
+   selects synchronous chart execution; generic task services remain available.
+
+Existing authentication, Task permissions and subscriber filtering still apply.
+Async charts also require the existing request opt-in, result format/type, cache,
+subscriber identity and Task-read permission conditions. Ineligible requests
+follow the synchronous path.
+
+| Deployment config | Effective Tasks UI flag | GAQ flag | Task services | Tasks UI | Chart path eligible for async |
+|---|---|---|---|---|---|
+| Off | Any | Any | Unavailable | Hidden | No (sync) |
+| On | Off | Off | Available | Hidden | No (sync) |
+| On | On | Off | Available | Visible | No (sync) |
+| On | Off | On | Available | Hidden | Yes |
+| On | On | On | Available | Visible | Yes |
+
+All availability remains subject to existing permissions. The standard
+FeatureFlagManager automatically derives effective GTF-on from GAQ-on, so enabling
+GAQ also shows the Tasks UI when infrastructure is installed. That derivation is
+unchanged. A customized resolver can return GAQ-on/GTF-off: async charts still work
+but the Tasks UI stays hidden. API polling/cancellation and MCP remain available.

Review Comment:
   **Suggestion:** A custom resolver returning GAQ on and GTF off still makes `GLOBAL_TASK_FRAMEWORK` enabled, so the Tasks UI remains visible.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Logic error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=16714e51b652428a8b80346622b79a04&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=16714e51b652428a8b80346622b79a04&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** docs/admin_docs/configuration/async-queries-celery.mdx
   **Line:** 147:148
   **Comment:**
   	*Logic Error: A custom resolver returning GAQ on and GTF off still makes `GLOBAL_TASK_FRAMEWORK` enabled, so the Tasks UI remains visible.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44426&comment_hash=92bff9807224e255841ea59aea40dc59408c6793ceb9516839b88253bdb8ff85&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44426&comment_hash=92bff9807224e255841ea59aea40dc59408c6793ceb9516839b88253bdb8ff85&reaction=dislike'>👎</a>



##########
UPDATING.md:
##########
@@ -24,6 +24,37 @@ assists people when migrating to a new version.
 
 ## Next
 
+### Task configuration and feature flags have separate responsibilities
+
+**Breaking change:** previously the `GLOBAL_TASK_FRAMEWORK` flag controlled task
+registration and admission. It controls **only the Tasks UI** after this change.
+It is not an execution/API kill switch or a security authorization control.
+
+- `GLOBAL_TASK_FRAMEWORK_ENABLED` (new deployment config, default `False`)
+  installs infrastructure and enables task admission, APIs including polling and
+  cancellation, and task MCP tools. Existing RBAC/subscriber restrictions remain.
+- `GLOBAL_TASK_FRAMEWORK` controls the Tasks menu/page/list, requiring config-on.
+- `GLOBAL_ASYNC_QUERIES` enables async chart eligibility, requiring config-on but
+  **not** the Tasks UI flag. GAQ-off selects sync without disabling generic tasks.
+
+Migrate task/GAQ deployments by setting `GLOBAL_TASK_FRAMEWORK_ENABLED = True` in
+`superset_config.py`. There is no fallback from static or dynamic flags. Use the
+same process-wide setting in web, worker, MCP and provisioning processes; run
+normal migrations and `superset init`, then restart all relevant processes.
+No permission-name or schema migration is introduced.
+
+Stock FeatureFlagManager still derives effective GTF-on from GAQ-on, so GAQ shows
+the Tasks UI with config-on. A custom resolver returning GAQ-on/GTF-off allows
+async charts with a hidden Tasks UI. Config-off makes task services/UI unavailable

Review Comment:
   **Suggestion:** A custom resolver cannot keep the Tasks UI hidden when GAQ is enabled because the resolver derives `GLOBAL_TASK_FRAMEWORK` on.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e1bc38bda04e4e7ab82a449318a9ff52&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e1bc38bda04e4e7ab82a449318a9ff52&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** UPDATING.md
   **Line:** 46:48
   **Comment:**
   	*Api Mismatch: A custom resolver cannot keep the Tasks UI hidden when GAQ is enabled because the resolver derives `GLOBAL_TASK_FRAMEWORK` on.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44426&comment_hash=a07df17e0a36b75dd6b607e15348342593d7fc4e0a9c4f00b5b78cffda16b892&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44426&comment_hash=a07df17e0a36b75dd6b607e15348342593d7fc4e0a9c4f00b5b78cffda16b892&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.

Antonio-RiveroMartnez commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5763234335

   > Quick comment: It feels confusing to have both a config flag `GLOBAL_TASK_FRAMEWORK_ENABLED` and a feature flag `GLOBAL_TASK_FRAMEWORK`, 
   
   Yes, indeed, not a big fan neither, but wanted to keep it as "backward compatible" as possible (even tho it's a breaking change regardless), so, open to suggestions.
   
   > what happens if only the feature flag is enabled, but not the config flag?
   
   you'll get a `GlobalTaskFrameworkDisabledError` and a logged a warning. 
   
   > decide on how and when config flags vs feature flags are used
   
   Totally, my two cents on this: `configs` should be for things that are OK to expect an app restart/re-deploy, while `Feature Flags` are for things that are expected to be dynamically available/hidden/blocked. That's why registering routes and thus permissions based on feature flags doesn't seem like a good pattern to maintain. I know this also happens for others like `SEMANTIC_LAYERS` and `ENABLE_EXTENSIONS`.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5729003531

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5762566880

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers an incremental AI Review.
   - `/review full` - Manually triggers a full AI Review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5763880184

   >  @rusackas we've discussed this before, but I feel now would maybe be a good time to take a step back and decide on how and when config flags vs feature flags are used.
   
   1000% agreed. Happy to bring it to the next Town Hall, or find a time to properly sync up and brainstorm a bit (a thread on Slack or finding a time to hop on a call and come up with some proposals)
   
   It definitely feels like we shouldn't need TWO flags to enable a feature. Maybe if something like GTF needs a server boot to be enabled, it should _only_ require that... the other feature flag could just see that it was enabled at boot and... enable it. Alternatively, anything that's needed at boot could be always-on, so the more conventional feature flag to enable/disable the feature as a runtime flag would "just work."


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.

Antonio-RiveroMartnez commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5764311396

   > Alternatively, anything that's needed at boot could be always-on, so the more conventional feature flag to enable/disable the feature as a runtime flag would "just work."
   
   Oh I like that, let me get rid of the extra config and instead just ensure boot is not affected and only have the FF as a gate in the UI/API requests.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5763075605

   Quick comment: It feels confusing to have both a config flag `GLOBAL_TASK_FRAMEWORK_ENABLED` and a feature flag `GLOBAL_TASK_FRAMEWORK`, especially if the feature flag is in fact only enabling the UI bits of the feature (the name implies it's the actual feature). Also, what happens if only the feature flag is enabled, but not the config flag? @rusackas we've discussed this before, but I feel now would maybe be a good time to take a step back and decide on how and when config flags vs feature flags are used.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): register infrastructure independently of runtime flags [superset]

Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.

Antonio-RiveroMartnez commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5774359914

   > I feel it's not a great solution from a security standpoint..
   
   Yes, I understand the drawbacks of supporting extra APIs in your app that you would never support or use and the security risk it might represent, however, I do believe we must find ways to not perpetuate this pattern of what's supposed to be runtime evaluated like a feature flag injected into boot sequence files.
   Not saying enabling the APIs is the best solution, probably the solution requires some changes in FAB etc, so yeah, even though I naively started the PR assuming it would have been an quick win, the way I see it, it's turning out to be something that requires more than just this PR.
   
   So, I'll close this for now, and we can use this thread or start a new one somewhere else to facilitate such broader discussion.
   
   Thanks both @villebro @rusackas 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5729066841

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44426?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `48.57143%` with `18 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 57.39%. Comparing base ([`a7dab4a`](https://app.codecov.io/gh/apache/superset/commit/a7dab4a5519cfc1cbfd2e2e1c83622cc1110ccde?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`42068fc`](https://app.codecov.io/gh/apache/superset/commit/42068fcbfa57b77a0695d7db0ad19ad5b4b0f846?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44426?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/initialization/\_\_init\_\_.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Finitialization%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | 25.00% | [0 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/tasks/api.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Ftasks%2Fapi.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdGFza3MvYXBpLnB5) | 62.50% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/commands/tasks/submit.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Fcommands%2Ftasks%2Fsubmit.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvdGFza3Mvc3VibWl0LnB5) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/app.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fapp.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvYXBwLnB5) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/task/tool/get\_task\_info.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Fmcp_service%2Ftask%2Ftool%2Fget_task_info.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvdGFzay90b29sL2dldF90YXNrX2luZm8ucHk=) | 60.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/task/tool/list\_tasks.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Fmcp_service%2Ftask%2Ftool%2Flist_tasks.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvdGFzay90b29sL2xpc3RfdGFza3MucHk=) | 60.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/tasks/decorators.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Ftasks%2Fdecorators.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdGFza3MvZGVjb3JhdG9ycy5weQ==) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/views/tasks.py](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&filepath=superset%2Fviews%2Ftasks.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdmlld3MvdGFza3MucHk=) | 50.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44426?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   > :exclamation:  There is a different number of reports uploaded between BASE (a7dab4a) and HEAD (42068fc). Click for more details.
   > 
   > <details><summary>HEAD has 6 uploads less than BASE</summary>
   >
   >| Flag | BASE (a7dab4a) | HEAD (42068fc) |
   >|------|------|------|
   >|python|8|2|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #44426       +/-   ##
   ===========================================
   - Coverage   80.57%   57.39%   -23.18%     
   ===========================================
     Files        2940     2940               
     Lines      175316   175339       +23     
     Branches    40696    40700        +4     
   ===========================================
   - Hits       141253   100636    -40617     
   - Misses      31398    73742    +42344     
   + Partials     2665      961     -1704     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.25% <48.57%> (+<0.01%)` | :arrow_up: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.14% <48.57%> (+<0.01%)` | :arrow_up: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.19% <48.57%> (-45.69%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/44426/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44426?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): register infrastructure independently of runtime flags [superset]

Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.

Antonio-RiveroMartnez closed pull request #44426: refactor(tasks): register infrastructure independently of runtime flags
URL: https://github.com/apache/superset/pull/44426


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on code in PR #44426:
URL: https://github.com/apache/superset/pull/44426#discussion_r4063786232


##########
tests/unit_tests/tasks/test_scheduler_executor.py:
##########
@@ -183,3 +183,35 @@ def test_persist_celery_task_id_is_noop_without_an_id() -> None:
 
     task.update_framework_private.assert_not_called()
     db.session.commit.assert_not_called()
+
+
+def test_admitted_worker_finishes_after_runtime_flag_is_disabled() -> None:
+    """Admission flags do not interrupt the worker's terminal transition."""
+    from superset.tasks.scheduler import _execute_task_body
+
+    native = uuid4()
+    task = MagicMock(uuid=native, status=TaskStatus.PENDING.value, properties_dict={})
+    ctx = MagicMock()
+    ctx.aborting_in_flight = False

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Vacuous mock context branch</b></div>
   <div id="fix">
   
   `ctx` is a bare `MagicMock`, so `ctx.fence_triggered` is a truthy Mock and the `finally` block in `_execute_task_body` takes the fence branch, not the abort branch. The test still passes because `TaskDAO.find_one_or_none` is mocked to return SUCCESS, masking which branch ran. Pin `fence_triggered`/`_abort_detected`/`timeout_triggered`/`abort_handlers_completed` like `_run_body_with_raising_executor` does so the SUCCESS path is actually exercised.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d0520b</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



##########
superset/initialization/__init__.py:
##########
@@ -460,17 +460,17 @@ def init_views(self) -> None:
             ),
         )
 
-        appbuilder.add_view(
-            TaskModelView,
-            "Tasks",
-            label=_("Tasks"),
-            icon="fa-clock-o",
-            category="Manage",
-            category_label=_("Manage"),
-            menu_cond=lambda: feature_flag_manager.is_feature_enabled(
-                "GLOBAL_TASK_FRAMEWORK"
-            ),
-        )
+        if self.config["GLOBAL_TASK_FRAMEWORK_ENABLED"]:
+            appbuilder.add_view(
+                TaskModelView,
+                "Tasks",
+                label=_("Tasks"),
+                icon="fa-clock-o",
+                category="Manage",
+                category_label=_("Manage"),
+                menu_cond=lambda: self.config["GLOBAL_TASK_FRAMEWORK_ENABLED"]
+                and feature_flag_manager.is_feature_enabled("GLOBAL_TASK_FRAMEWORK"),

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Redundant menu_cond condition</b></div>
   <div id="fix">
   
   The `add_view` call is already inside `if self.config["GLOBAL_TASK_FRAMEWORK_ENABLED"]:` (line 463), so the lambda's first operand is always True when `menu_cond` runs — the `and` adds dead logic. This config is documented restart-static (config.py:2978-2982), so it cannot legitimately flip at runtime; keep only the `feature_flag_manager.is_feature_enabled("GLOBAL_TASK_FRAMEWORK")` check in the lambda.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d0520b</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



##########
superset/initialization/__init__.py:
##########
@@ -1667,10 +1667,25 @@ def configure_wtf(self) -> None:
 
     def configure_task_manager(self) -> None:
         """Initialize the TaskManager for GTF realtime notifications."""
-        if feature_flag_manager.is_feature_enabled("GLOBAL_TASK_FRAMEWORK"):
+        if self.config["GLOBAL_TASK_FRAMEWORK_ENABLED"]:
             from superset.tasks.manager import TaskManager
 
             TaskManager.init_app(self.superset_app)
+        else:
+            static_flags = {
+                **self.config["DEFAULT_FEATURE_FLAGS"],
+                **self.config["FEATURE_FLAGS"],
+            }

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing local type annotation</b></div>
   <div id="fix">
   
   BITO rule 13598 asks for explicit annotations on local and derived variables; sibling code at lines 1005-1006 annotates the identical DEFAULT_FEATURE_FLAGS/FEATURE_FLAGS merge as `dict[str, Any]`. Annotate `static_flags` the same way for consistency and static type coverage.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d0520b</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



##########
superset-frontend/src/utils/asyncMode.test.ts:
##########
@@ -51,7 +56,9 @@ test('never async when the feature flag is off', () => {
 
 test('falls back to the deployment default when no override', () => {
   mockFeatureEnabled.mockImplementation(
-    f => f === FeatureFlag.GlobalAsyncQueries,
+    f =>
+      f === FeatureFlag.GlobalAsyncQueries ||
+      f === FeatureFlag.GlobalTaskFramework,

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Triplicated flag mock predicate</b></div>
   <div id="fix">
   
   The two-flag predicate `f === FeatureFlag.GlobalAsyncQueries || f === FeatureFlag.GlobalTaskFramework` is now repeated verbatim in three tests (also lines 72-74 and 94-96). Extract one shared `mockImplementation` helper so a future flag change updates a single site instead of three copies that can silently drift apart.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d0520b</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): separate deployment registration from runtime flags [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5729020885

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 42068fcbfa57b77a0695d7db0ad19ad5b4b0f846 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aad18ca1ac07a00088a4927 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44426--superset-docs-preview.netlify.app](https://deploy-preview-44426--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ0NDI2LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.Vr_ytuspC3K7e8RK3ivDQLx4jevS1wbQYPwsIG2ZyoA)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8w6QSPSukHHmMZPgqnYtIjo7rl7i_dCbeWsDCvAAHvDTEd3YC_anTmeTlDJPgZx1RS5H-tqKs9m-5oRrTKECklqlFIag9Myavh-) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-44426--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor(tasks): register infrastructure independently of runtime flags [superset]

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro commented on PR #44426:
URL: https://github.com/apache/superset/pull/44426#issuecomment-5766719630

   The problem with enabling infra for all feature-flagged features is they might open up security vulnerabilities during evaluation, hence why IMO we need to gate these. So while I agree that would be a convenient solution for this, I feel it's not a great solution from a security standpoint..


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org