Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add new SQLLAB_FORCE_RUN_ASYNC feature flag #29231

Merged
merged 8 commits into from
Jun 20, 2024
Merged

Conversation

mistercrunch
Copy link
Member

When setting up a celery backend to run asynchronous workloads, the administrator can set any given database connection to use (or not use) that backend. This PR introduced a new feature flag SQLLAB_FORCE_RUN_ASYNC (False by default) that disregards the database configuration, and forces SQL Lab to use the async backend.

NOTE: superset/sqllab/sqllab_execution_context.py didn't have unit tests so I wrote started a new module to run basic tests around it.

@dosubot dosubot bot added global:async-query Related to Async Queries feature need:tests This PR requires tests sqllab Namespace | Anything related to the SQL Lab labels Jun 12, 2024
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 12, 2024
@john-bodley
Copy link
Member

@mistercrunch could you provide more details with regards to the motivation for this change? It likely introduces a degraded UX as admins likely could be misinformed that a given database is running in sync mode whereas in actuality (when the feature flag is enabled) the database is using async.

@mistercrunch
Copy link
Member Author

mistercrunch commented Jun 13, 2024

The requirement is we want the optionality to enforce this for some of our customers at Preset, but I get your point and I agree that I should probably disable the UI element in the database connection form.

When setting up a celery backend to run asynchronous workloads, the administrator can set any given database connection to use (or not use) that backend. This PR introduced a new feature flag SQLLAB_FORCE_RUN_ASYNC (False by default) that disregards the database configuration, and forces SQL Lab to use the async backend.

NOTE: superset/sqllab/sqllab_execution_context.py didn't have unit tests so I wrote started a new module to run basic tests around it.
@mistercrunch
Copy link
Member Author

@john-bodley I added frontend cues ->
Screenshot 2024-06-18 at 12 09 38 PM

@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label Jun 18, 2024
@github-actions github-actions bot removed the github_actions Pull requests that update GitHub Actions code label Jun 18, 2024
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Copy link
Member

@geido geido left a comment

Choose a reason for hiding this comment

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

Sorry my suggestion wasn't properly indented. However, this looks good!

@mistercrunch
Copy link
Member Author

No worries, a quick --fix took care of it

@mistercrunch mistercrunch merged commit 5e060ce into master Jun 20, 2024
34 checks passed
@rusackas rusackas deleted the ff_force_async branch June 20, 2024 21:44
eschutho pushed a commit that referenced this pull request Jul 24, 2024
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
global:async-query Related to Async Queries feature need:tests This PR requires tests packages size/L sqllab Namespace | Anything related to the SQL Lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants